Patchwork [10/20] udev-cache: parametrize sysconf file paths

login
register
mail settings
Submitter Ben Shelton
Date Aug. 4, 2014, 6:41 p.m.
Message ID <17ffe840a21d09f0dcb1cdfeb13d9d21331055f9.1407177403.git.ben.shelton@ni.com>
Download mbox | patch
Permalink /patch/77229/
State Accepted
Commit fd0aa6af628247d41fc61ba84be748f2f4fc2a72
Headers show

Comments

Ben Shelton - Aug. 4, 2014, 6:41 p.m.
From: Richard Tollerton <rich.tollerton@ni.com>

The udev-cache facility uses files that represent system states, to
ensure that the cache tarball is valid to apply. These paths were
hardcoded in several places; collect them into DEVCACHE_SYSCONF and
DEVCACHE_CURRENT_SYSCONF in the defaults file.

Natinst-Rally-ID: TA44427
Acked-by: Gratian Crisan <gratian.crisan@ni.com>
Natinst-ReviewBoard-ID: 58620
Signed-off-by: Richard Tollerton <rich.tollerton@ni.com>
---
 meta/recipes-core/udev/udev/init               | 8 ++++----
 meta/recipes-core/udev/udev/udev-cache         | 4 ++--
 meta/recipes-core/udev/udev/udev-cache.default | 4 ++++
 3 files changed, 10 insertions(+), 6 deletions(-)
Otavio Salvador - Aug. 4, 2014, 7:44 p.m.
On Mon, Aug 4, 2014 at 3:41 PM, Ben Shelton <ben.shelton@ni.com> wrote:
> From: Richard Tollerton <rich.tollerton@ni.com>
>
> The udev-cache facility uses files that represent system states, to
> ensure that the cache tarball is valid to apply. These paths were
> hardcoded in several places; collect them into DEVCACHE_SYSCONF and
> DEVCACHE_CURRENT_SYSCONF in the defaults file.
>
> Natinst-Rally-ID: TA44427
> Acked-by: Gratian Crisan <gratian.crisan@ni.com>
> Natinst-ReviewBoard-ID: 58620
> Signed-off-by: Richard Tollerton <rich.tollerton@ni.com>
> ---
...
> --- a/meta/recipes-core/udev/udev/udev-cache
> +++ b/meta/recipes-core/udev/udev/udev-cache
> @@ -23,12 +23,12 @@ if [ "$ROOTFS_READ_ONLY" = "yes" ]; then
>      exit 0
>  fi
>
> -if [ "$DEVCACHE" != "" -a -e /dev/shm/udev.cache ]; then
> +if [ "$DEVCACHE" != "" -a -e "$DEVCACHE_CURRENT_SYSCONF" ]; then
>         [ "${VERBOSE}" != "no" ] && echo "Populating dev cache"
>         (
>                 tar czf "${DEVCACHE}.tmp" dev -C / --exclude=log
>                 mv -f "${DEVCACHE}.tmp" "$DEVCACHE"
> -               mv /dev/shm/udev.cache /etc/udev/cache.data
> +               mv "$DEVCACHE_CURRENT_SYSCONF" "$DEVCACHE_SYSCONF"
>         ) &
>  fi
>
> diff --git a/meta/recipes-core/udev/udev/udev-cache.default b/meta/recipes-core/udev/udev/udev-cache.default
> index 656c2a4..7f39a68 100644
> --- a/meta/recipes-core/udev/udev/udev-cache.default
> +++ b/meta/recipes-core/udev/udev/udev-cache.default
> @@ -2,4 +2,8 @@
>
>  # Comment this out to disable device cache
>  DEVCACHE="/etc/udev-cache.tar.gz"
> +
> +DEVCACHE_SYSCONF="/etc/udev/cache.data"
> +DEVCACHE_CURRENT_SYSCONF="/dev/shm/udev.cache"
> +
>  PROBE_PLATFORM_BUS="yes"
> --

Most users won't need to change this so I think this should have a
default value in the script and don't be added in the default.
Ben Shelton - Aug. 4, 2014, 9:38 p.m.
On 08/04, Otavio Salvador wrote:
> On Mon, Aug 4, 2014 at 3:41 PM, Ben Shelton <ben.shelton@ni.com> wrote:
> > From: Richard Tollerton <rich.tollerton@ni.com>
> >
> > The udev-cache facility uses files that represent system states, to
> > ensure that the cache tarball is valid to apply. These paths were
> > hardcoded in several places; collect them into DEVCACHE_SYSCONF and
> > DEVCACHE_CURRENT_SYSCONF in the defaults file.
> >
> > Natinst-Rally-ID: TA44427
> > Acked-by: Gratian Crisan <gratian.crisan@ni.com>
> > Natinst-ReviewBoard-ID: 58620
> > Signed-off-by: Richard Tollerton <rich.tollerton@ni.com>
> > ---
> ...
> > --- a/meta/recipes-core/udev/udev/udev-cache
> > +++ b/meta/recipes-core/udev/udev/udev-cache
> > @@ -23,12 +23,12 @@ if [ "$ROOTFS_READ_ONLY" = "yes" ]; then
> >      exit 0
> >  fi
> >
> > -if [ "$DEVCACHE" != "" -a -e /dev/shm/udev.cache ]; then
> > +if [ "$DEVCACHE" != "" -a -e "$DEVCACHE_CURRENT_SYSCONF" ]; then
> >         [ "${VERBOSE}" != "no" ] && echo "Populating dev cache"
> >         (
> >                 tar czf "${DEVCACHE}.tmp" dev -C / --exclude=log
> >                 mv -f "${DEVCACHE}.tmp" "$DEVCACHE"
> > -               mv /dev/shm/udev.cache /etc/udev/cache.data
> > +               mv "$DEVCACHE_CURRENT_SYSCONF" "$DEVCACHE_SYSCONF"
> >         ) &
> >  fi
> >
> > diff --git a/meta/recipes-core/udev/udev/udev-cache.default b/meta/recipes-core/udev/udev/udev-cache.default
> > index 656c2a4..7f39a68 100644
> > --- a/meta/recipes-core/udev/udev/udev-cache.default
> > +++ b/meta/recipes-core/udev/udev/udev-cache.default
> > @@ -2,4 +2,8 @@
> >
> >  # Comment this out to disable device cache
> >  DEVCACHE="/etc/udev-cache.tar.gz"
> > +
> > +DEVCACHE_SYSCONF="/etc/udev/cache.data"
> > +DEVCACHE_CURRENT_SYSCONF="/dev/shm/udev.cache"
> > +
> >  PROBE_PLATFORM_BUS="yes"
> > --
> 
> Most users won't need to change this so I think this should have a
> default value in the script and don't be added in the default.
> 

My concern is that the uses of both DEVCACHE_SYSCONF and
DEVCACHE_CURRENT_SYSCONF are split across two script files: udev/init
and udev/udev-cache.  I don't like replicating the same default setting
in both, but if that's preferable to adding it in the default, that's
OK.

Ben


> -- 
> Otavio Salvador                             O.S. Systems
> http://www.ossystems.com.br        http://code.ossystems.com.br
> Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750
Otavio Salvador - Aug. 4, 2014, 9:42 p.m.
On Mon, Aug 4, 2014 at 6:38 PM, Ben Shelton <ben.shelton@ni.com> wrote:
> On 08/04, Otavio Salvador wrote:
>> On Mon, Aug 4, 2014 at 3:41 PM, Ben Shelton <ben.shelton@ni.com> wrote:
>> > From: Richard Tollerton <rich.tollerton@ni.com>
>> >
>> > The udev-cache facility uses files that represent system states, to
>> > ensure that the cache tarball is valid to apply. These paths were
>> > hardcoded in several places; collect them into DEVCACHE_SYSCONF and
>> > DEVCACHE_CURRENT_SYSCONF in the defaults file.
>> >
>> > Natinst-Rally-ID: TA44427
>> > Acked-by: Gratian Crisan <gratian.crisan@ni.com>
>> > Natinst-ReviewBoard-ID: 58620
>> > Signed-off-by: Richard Tollerton <rich.tollerton@ni.com>
>> > ---
>> ...
>> > --- a/meta/recipes-core/udev/udev/udev-cache
>> > +++ b/meta/recipes-core/udev/udev/udev-cache
>> > @@ -23,12 +23,12 @@ if [ "$ROOTFS_READ_ONLY" = "yes" ]; then
>> >      exit 0
>> >  fi
>> >
>> > -if [ "$DEVCACHE" != "" -a -e /dev/shm/udev.cache ]; then
>> > +if [ "$DEVCACHE" != "" -a -e "$DEVCACHE_CURRENT_SYSCONF" ]; then
>> >         [ "${VERBOSE}" != "no" ] && echo "Populating dev cache"
>> >         (
>> >                 tar czf "${DEVCACHE}.tmp" dev -C / --exclude=log
>> >                 mv -f "${DEVCACHE}.tmp" "$DEVCACHE"
>> > -               mv /dev/shm/udev.cache /etc/udev/cache.data
>> > +               mv "$DEVCACHE_CURRENT_SYSCONF" "$DEVCACHE_SYSCONF"
>> >         ) &
>> >  fi
>> >
>> > diff --git a/meta/recipes-core/udev/udev/udev-cache.default b/meta/recipes-core/udev/udev/udev-cache.default
>> > index 656c2a4..7f39a68 100644
>> > --- a/meta/recipes-core/udev/udev/udev-cache.default
>> > +++ b/meta/recipes-core/udev/udev/udev-cache.default
>> > @@ -2,4 +2,8 @@
>> >
>> >  # Comment this out to disable device cache
>> >  DEVCACHE="/etc/udev-cache.tar.gz"
>> > +
>> > +DEVCACHE_SYSCONF="/etc/udev/cache.data"
>> > +DEVCACHE_CURRENT_SYSCONF="/dev/shm/udev.cache"
>> > +
>> >  PROBE_PLATFORM_BUS="yes"
>> > --
>>
>> Most users won't need to change this so I think this should have a
>> default value in the script and don't be added in the default.
>>
>
> My concern is that the uses of both DEVCACHE_SYSCONF and
> DEVCACHE_CURRENT_SYSCONF are split across two script files: udev/init
> and udev/udev-cache.  I don't like replicating the same default setting
> in both, but if that's preferable to adding it in the default, that's
> OK.

I think this also provides a backward compatibility for users
providing their own default file. Otherwise you'll break those.
Ben Shelton - Aug. 4, 2014, 9:48 p.m.
On 08/04, Otavio Salvador wrote:
> On Mon, Aug 4, 2014 at 6:38 PM, Ben Shelton <ben.shelton@ni.com> wrote:
> > On 08/04, Otavio Salvador wrote:
> >> On Mon, Aug 4, 2014 at 3:41 PM, Ben Shelton <ben.shelton@ni.com> wrote:
> >> > From: Richard Tollerton <rich.tollerton@ni.com>
> >> >
> >> > The udev-cache facility uses files that represent system states, to
> >> > ensure that the cache tarball is valid to apply. These paths were
> >> > hardcoded in several places; collect them into DEVCACHE_SYSCONF and
> >> > DEVCACHE_CURRENT_SYSCONF in the defaults file.
> >> >
> >> > Natinst-Rally-ID: TA44427
> >> > Acked-by: Gratian Crisan <gratian.crisan@ni.com>
> >> > Natinst-ReviewBoard-ID: 58620
> >> > Signed-off-by: Richard Tollerton <rich.tollerton@ni.com>
> >> > ---
> >> ...
> >> > --- a/meta/recipes-core/udev/udev/udev-cache
> >> > +++ b/meta/recipes-core/udev/udev/udev-cache
> >> > @@ -23,12 +23,12 @@ if [ "$ROOTFS_READ_ONLY" = "yes" ]; then
> >> >      exit 0
> >> >  fi
> >> >
> >> > -if [ "$DEVCACHE" != "" -a -e /dev/shm/udev.cache ]; then
> >> > +if [ "$DEVCACHE" != "" -a -e "$DEVCACHE_CURRENT_SYSCONF" ]; then
> >> >         [ "${VERBOSE}" != "no" ] && echo "Populating dev cache"
> >> >         (
> >> >                 tar czf "${DEVCACHE}.tmp" dev -C / --exclude=log
> >> >                 mv -f "${DEVCACHE}.tmp" "$DEVCACHE"
> >> > -               mv /dev/shm/udev.cache /etc/udev/cache.data
> >> > +               mv "$DEVCACHE_CURRENT_SYSCONF" "$DEVCACHE_SYSCONF"
> >> >         ) &
> >> >  fi
> >> >
> >> > diff --git a/meta/recipes-core/udev/udev/udev-cache.default b/meta/recipes-core/udev/udev/udev-cache.default
> >> > index 656c2a4..7f39a68 100644
> >> > --- a/meta/recipes-core/udev/udev/udev-cache.default
> >> > +++ b/meta/recipes-core/udev/udev/udev-cache.default
> >> > @@ -2,4 +2,8 @@
> >> >
> >> >  # Comment this out to disable device cache
> >> >  DEVCACHE="/etc/udev-cache.tar.gz"
> >> > +
> >> > +DEVCACHE_SYSCONF="/etc/udev/cache.data"
> >> > +DEVCACHE_CURRENT_SYSCONF="/dev/shm/udev.cache"
> >> > +
> >> >  PROBE_PLATFORM_BUS="yes"
> >> > --
> >>
> >> Most users won't need to change this so I think this should have a
> >> default value in the script and don't be added in the default.
> >>
> >
> > My concern is that the uses of both DEVCACHE_SYSCONF and
> > DEVCACHE_CURRENT_SYSCONF are split across two script files: udev/init
> > and udev/udev-cache.  I don't like replicating the same default setting
> > in both, but if that's preferable to adding it in the default, that's
> > OK.
> 
> I think this also provides a backward compatibility for users
> providing their own default file. Otherwise you'll break those.

Fair point.  I'll make the change.

Thanks,
Ben

> 
> -- 
> Otavio Salvador                             O.S. Systems
> http://www.ossystems.com.br        http://code.ossystems.com.br
> Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Patch

diff --git a/meta/recipes-core/udev/udev/init b/meta/recipes-core/udev/udev/init
index bcb9040..5db029a 100644
--- a/meta/recipes-core/udev/udev/init
+++ b/meta/recipes-core/udev/udev/init
@@ -66,13 +66,13 @@  case "$1" in
             if [ -e $DEVCACHE ]; then
 		    readfiles $CMP_FILE_LIST
 		    NEWDATA="$READDATA"
-		    readfiles /etc/udev/cache.data
+		    readfiles "$DEVCACHE_SYSCONF"
 		    OLDDATA="$READDATA"
 		    if [ "$OLDDATA" = "$NEWDATA" ]; then
                             tar xzf $DEVCACHE -C / --exclude=log
                             not_first_boot=1
                             [ "$VERBOSE" != "no" ] && echo "udev: using cache file $DEVCACHE"
-                            [ -e /dev/shm/udev.cache ] && rm -f /dev/shm/udev.cache
+                            [ -e "$DEVCACHE_CURRENT_SYSCONF" ] && rm -f "$DEVCACHE_CURRENT_SYSCONF"
                     else
 			    # Output detailed reason why the cached /dev is not used
 			    if [ "$VERBOSE" != "no" ]; then
@@ -81,14 +81,14 @@  case "$1" in
 				    echo "udev: olddata: $OLDDATA"
 				    echo "udev: newdata: $NEWDATA"
 			    fi
-			    echo "$NEWDATA" > /dev/shm/udev.cache
+			    echo "$NEWDATA" > "$DEVCACHE_CURRENT_SYSCONF"
                     fi
 	    else
 		    if [ "$ROOTFS_READ_ONLY" != "yes" ]; then
 			    # If rootfs is not read-only, it's possible that a new udev cache would be generated;
 			    # otherwise, we do not bother to read files.
 			    readfiles $CMP_FILE_LIST
-			    echo "$READDATA" > /dev/shm/udev.cache
+			    echo "$READDATA" > "$DEVCACHE_CURRENT_SYSCONF"
 		    fi
             fi
     fi
diff --git a/meta/recipes-core/udev/udev/udev-cache b/meta/recipes-core/udev/udev/udev-cache
index e730608..7406e07 100644
--- a/meta/recipes-core/udev/udev/udev-cache
+++ b/meta/recipes-core/udev/udev/udev-cache
@@ -23,12 +23,12 @@  if [ "$ROOTFS_READ_ONLY" = "yes" ]; then
     exit 0
 fi
 
-if [ "$DEVCACHE" != "" -a -e /dev/shm/udev.cache ]; then
+if [ "$DEVCACHE" != "" -a -e "$DEVCACHE_CURRENT_SYSCONF" ]; then
 	[ "${VERBOSE}" != "no" ] && echo "Populating dev cache"
 	(
 		tar czf "${DEVCACHE}.tmp" dev -C / --exclude=log
 		mv -f "${DEVCACHE}.tmp" "$DEVCACHE"
-		mv /dev/shm/udev.cache /etc/udev/cache.data
+		mv "$DEVCACHE_CURRENT_SYSCONF" "$DEVCACHE_SYSCONF"
 	) &
 fi
 
diff --git a/meta/recipes-core/udev/udev/udev-cache.default b/meta/recipes-core/udev/udev/udev-cache.default
index 656c2a4..7f39a68 100644
--- a/meta/recipes-core/udev/udev/udev-cache.default
+++ b/meta/recipes-core/udev/udev/udev-cache.default
@@ -2,4 +2,8 @@ 
 
 # Comment this out to disable device cache
 DEVCACHE="/etc/udev-cache.tar.gz"
+
+DEVCACHE_SYSCONF="/etc/udev/cache.data"
+DEVCACHE_CURRENT_SYSCONF="/dev/shm/udev.cache"
+
 PROBE_PLATFORM_BUS="yes"