Patchwork [08/12] initscripts: let populate-volatile.sh create the /tmp link

login
mail settings
Submitter Qi.Chen@windriver.com
Date Jan. 24, 2013, 8:16 a.m.
Message ID <202617fa712665192de55285dc8ea3f4699315bc.1359014474.git.Qi.Chen@windriver.com>
Download mbox | patch
Permalink /patch/43255/
State Accepted
Commit 12c4acd7ac5a27cf3676065b60f1c8395c96854c
Headers show

Comments

Qi.Chen@windriver.com - Jan. 24, 2013, 8:16 a.m.
From: Chen Qi <Qi.Chen@windriver.com>

Previously, the /tmp link (/tmp -> /var/tmp) was created by the
bootmisc.sh script. So in case of a read-only rootfs, this symlink
would not be created correctly.

The populate-volatile.sh script is intended to handle all directories
and files related to volatile storage, so we should let it create
the /tmp link.

In addition, because of the improments of populate-volatile.sh, the data
loss problem of bug#3404 is also resolved by this patch.

[YOCTO #3406]
[YOCTO #3404]

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 .../initscripts/initscripts-1.0/bootmisc.sh        |    9 +--------
 .../initscripts/initscripts-1.0/volatiles          |    1 +
 2 files changed, 2 insertions(+), 8 deletions(-)
Mark Hatle - Jan. 24, 2013, 4:11 p.m.
On 1/24/13 2:16 AM, Qi.Chen@windriver.com wrote:
> From: Chen Qi <Qi.Chen@windriver.com>
>
> Previously, the /tmp link (/tmp -> /var/tmp) was created by the
> bootmisc.sh script. So in case of a read-only rootfs, this symlink
> would not be created correctly.
>
> The populate-volatile.sh script is intended to handle all directories
> and files related to volatile storage, so we should let it create
> the /tmp link.
>
> In addition, because of the improments of populate-volatile.sh, the data
> loss problem of bug#3404 is also resolved by this patch.
>
> [YOCTO #3406]
> [YOCTO #3404]
>
> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> ---
>   .../initscripts/initscripts-1.0/bootmisc.sh        |    9 +--------
>   .../initscripts/initscripts-1.0/volatiles          |    1 +
>   2 files changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/meta/recipes-core/initscripts/initscripts-1.0/bootmisc.sh b/meta/recipes-core/initscripts/initscripts-1.0/bootmisc.sh
> index 4f76cb4..3b5a47f 100755
> --- a/meta/recipes-core/initscripts/initscripts-1.0/bootmisc.sh
> +++ b/meta/recipes-core/initscripts/initscripts-1.0/bootmisc.sh
> @@ -54,14 +54,7 @@ fi
>
>   #
>   # This is as good a place as any for a sanity check
> -# /tmp should be a symlink to /var/tmp to cut down on the number
> -# of mounted ramdisks.
> -if test ! -L /tmp && test -d /var/tmp
> -then
> -	rm -rf /tmp
> -	ln -sf /var/tmp /tmp
> -fi
> -

I saw this the other day.  It seems like the wrong solution, if the expectation 
is that all systems want this linked behavior.  Instead the link should be 
configured in the meta/files/fs-perms.txt

Something like:

-/tmp                            01777   root    root    false - - -
+/tmp                            link    /var/tmp

Making that change would avoid this patch, and then all of the filesystems could 
inherit this behavior.  (Other distributions that don't want it can provide 
their own custom fs-perms.txt file, and simply set the older value, which will 
append to the default.)

> +#
>   # Set the system clock from hardware clock
>   # If the timestamp is more recent than the current time,
>   # use the timestamp instead.
> diff --git a/meta/recipes-core/initscripts/initscripts-1.0/volatiles b/meta/recipes-core/initscripts/initscripts-1.0/volatiles
> index e0741aa..f7e2ef7 100644
> --- a/meta/recipes-core/initscripts/initscripts-1.0/volatiles
> +++ b/meta/recipes-core/initscripts/initscripts-1.0/volatiles
> @@ -31,6 +31,7 @@ l root root 1777 /var/lock /var/volatile/lock
>   l root root 0755 /var/log /var/volatile/log
>   l root root 0755 /var/run /var/volatile/run
>   l root root 1777 /var/tmp /var/volatile/tmp
> +l root root 1777 /tmp /var/tmp
>   d root root 0755 /var/lock/subsys none
>   f root root 0664 /var/log/wtmp none
>   f root root 0664 /var/run/utmp none
>
Mark Hatle - Jan. 24, 2013, 5:39 p.m.
On 1/24/13 2:16 AM, Qi.Chen@windriver.com wrote:
> From: Chen Qi <Qi.Chen@windriver.com>
>
> Previously, the /tmp link (/tmp -> /var/tmp) was created by the
> bootmisc.sh script. So in case of a read-only rootfs, this symlink
> would not be created correctly.
>
> The populate-volatile.sh script is intended to handle all directories
> and files related to volatile storage, so we should let it create
> the /tmp link.
>
> In addition, because of the improments of populate-volatile.sh, the data
> loss problem of bug#3404 is also resolved by this patch.
>
> [YOCTO #3406]
> [YOCTO #3404]
>
> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> ---
>   .../initscripts/initscripts-1.0/bootmisc.sh        |    9 +--------
>   .../initscripts/initscripts-1.0/volatiles          |    1 +
>   2 files changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/meta/recipes-core/initscripts/initscripts-1.0/bootmisc.sh b/meta/recipes-core/initscripts/initscripts-1.0/bootmisc.sh
> index 4f76cb4..3b5a47f 100755
> --- a/meta/recipes-core/initscripts/initscripts-1.0/bootmisc.sh
> +++ b/meta/recipes-core/initscripts/initscripts-1.0/bootmisc.sh
> @@ -54,14 +54,7 @@ fi
>
>   #
>   # This is as good a place as any for a sanity check
> -# /tmp should be a symlink to /var/tmp to cut down on the number
> -# of mounted ramdisks.
> -if test ! -L /tmp && test -d /var/tmp
> -then
> -	rm -rf /tmp
> -	ln -sf /var/tmp /tmp
> -fi
> -

I saw this the other day.  It seems like the wrong solution, if the expectation 
is that all systems want this linked behavior.  Instead the link should be 
configured in the meta/files/fs-perms.txt

Something like:

-/tmp                            01777   root    root    false - - -
+/tmp                            link    /var/tmp

Making that change would avoid this patch, and then all of the filesystems could 
inherit this behavior.  (Other distributions that don't want it can provide 
their own custom fs-perms.txt file, and simply set the older value, which will 
append to the default.)

> +#
>   # Set the system clock from hardware clock
>   # If the timestamp is more recent than the current time,
>   # use the timestamp instead.
> diff --git a/meta/recipes-core/initscripts/initscripts-1.0/volatiles b/meta/recipes-core/initscripts/initscripts-1.0/volatiles
> index e0741aa..f7e2ef7 100644
> --- a/meta/recipes-core/initscripts/initscripts-1.0/volatiles
> +++ b/meta/recipes-core/initscripts/initscripts-1.0/volatiles
> @@ -31,6 +31,7 @@ l root root 1777 /var/lock /var/volatile/lock
>   l root root 0755 /var/log /var/volatile/log
>   l root root 0755 /var/run /var/volatile/run
>   l root root 1777 /var/tmp /var/volatile/tmp
> +l root root 1777 /tmp /var/tmp
>   d root root 0755 /var/lock/subsys none
>   f root root 0664 /var/log/wtmp none
>   f root root 0664 /var/run/utmp none
>
Qi.Chen@windriver.com - Jan. 25, 2013, 6:17 a.m.
On 01/25/2013 01:39 AM, Mark Hatle wrote:
> On 1/24/13 2:16 AM, Qi.Chen@windriver.com wrote:
>> From: Chen Qi <Qi.Chen@windriver.com>
>>
>> Previously, the /tmp link (/tmp -> /var/tmp) was created by the
>> bootmisc.sh script. So in case of a read-only rootfs, this symlink
>> would not be created correctly.
>>
>> The populate-volatile.sh script is intended to handle all directories
>> and files related to volatile storage, so we should let it create
>> the /tmp link.
>>
>> In addition, because of the improments of populate-volatile.sh, the data
>> loss problem of bug#3404 is also resolved by this patch.
>>
>> [YOCTO #3406]
>> [YOCTO #3404]
>>
>> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
>> ---
>>   .../initscripts/initscripts-1.0/bootmisc.sh        |    9 +--------
>>   .../initscripts/initscripts-1.0/volatiles          |    1 +
>>   2 files changed, 2 insertions(+), 8 deletions(-)
>>
>> diff --git 
>> a/meta/recipes-core/initscripts/initscripts-1.0/bootmisc.sh 
>> b/meta/recipes-core/initscripts/initscripts-1.0/bootmisc.sh
>> index 4f76cb4..3b5a47f 100755
>> --- a/meta/recipes-core/initscripts/initscripts-1.0/bootmisc.sh
>> +++ b/meta/recipes-core/initscripts/initscripts-1.0/bootmisc.sh
>> @@ -54,14 +54,7 @@ fi
>>
>>   #
>>   # This is as good a place as any for a sanity check
>> -# /tmp should be a symlink to /var/tmp to cut down on the number
>> -# of mounted ramdisks.
>> -if test ! -L /tmp && test -d /var/tmp
>> -then
>> -    rm -rf /tmp
>> -    ln -sf /var/tmp /tmp
>> -fi
>> -
>
> I saw this the other day.  It seems like the wrong solution, if the 
> expectation is that all systems want this linked behavior. Instead the 
> link should be configured in the meta/files/fs-perms.txt
>
> Something like:
>
> -/tmp                            01777   root    root    false - - -
> +/tmp                            link    /var/tmp
>
> Making that change would avoid this patch, and then all of the 
> filesystems could inherit this behavior.  (Other distributions that 
> don't want it can provide their own custom fs-perms.txt file, and 
> simply set the older value, which will append to the default.)
>

Thank you for your careful review :)

Here's a little more explanation on this patch. (Correct me if I'm wrong 
somewhere.)

No matter we're going to change the fs-perms.txt file or not, this patch 
is needed for things to work correctly. The populate-volatile.sh in 
sysvinit scripts, together with its configuration files, are a final 
means to ensure the directories, files and links related to volatile 
storage are created correctly.

There are link config items in fs-perms.txt.
${localstatedir}/cache    link    volatile/cache
${localstatedir}/run    link    volatile/run
${localstatedir}/log    link    volatile/log
${localstatedir}/lock    link    volatile/lock
${localstatedir}/tmp    link    volatile/tmp
There configurations are great and very reasonable, because they are 
common for different init systems.

But I'm not very sure about this link config item -- "/tmp link    
/var/tmp "

I want to restrict the creation of this link (/tmp -> /var/tmp) in the 
sysvinit scripts, because in this way, it doesn't affect other init 
systems such as systemd. The same strategy goes with the /run directory.

Here's the output from the mount command from a systemd-based 
core-image-minimal image:
root@qemux86:~# mount | grep tmpfs
devtmpfs on /dev type devtmpfs 
(rw,relatime,size=59032k,nr_inodes=14758,mode=755)
tmpfs on /dev/shm type tmpfs (rw,nosuid,nodev)
tmpfs on /run type tmpfs (rw,nosuid,nodev,mode=755)
tmpfs on /sys/fs/cgroup type tmpfs (rw,nosuid,nodev,noexec,mode=755)
tmpfs on /tmp type tmpfs (rw)
tmpfs on /var/volatile type tmpfs (rw,relatime)
tmpfs on /media/ram type tmpfs (rw,relatime)

Here's the output form the mount command from a sysv-based 
core-image-minimal image:
root@qemux86:~# mount | grep tmpfs
devtmpfs on /dev type devtmpfs 
(rw,relatime,size=59032k,nr_inodes=14758,mode=755)
tmpfs on /var/volatile type tmpfs (rw,relatime)
tmpfs on /media/ram type tmpfs (rw,relatime)

Best Regards,
Chen Qi

>> +#
>>   # Set the system clock from hardware clock
>>   # If the timestamp is more recent than the current time,
>>   # use the timestamp instead.
>> diff --git a/meta/recipes-core/initscripts/initscripts-1.0/volatiles 
>> b/meta/recipes-core/initscripts/initscripts-1.0/volatiles
>> index e0741aa..f7e2ef7 100644
>> --- a/meta/recipes-core/initscripts/initscripts-1.0/volatiles
>> +++ b/meta/recipes-core/initscripts/initscripts-1.0/volatiles
>> @@ -31,6 +31,7 @@ l root root 1777 /var/lock /var/volatile/lock
>>   l root root 0755 /var/log /var/volatile/log
>>   l root root 0755 /var/run /var/volatile/run
>>   l root root 1777 /var/tmp /var/volatile/tmp
>> +l root root 1777 /tmp /var/tmp
>>   d root root 0755 /var/lock/subsys none
>>   f root root 0664 /var/log/wtmp none
>>   f root root 0664 /var/run/utmp none
>>
>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>
>

Patch

diff --git a/meta/recipes-core/initscripts/initscripts-1.0/bootmisc.sh b/meta/recipes-core/initscripts/initscripts-1.0/bootmisc.sh
index 4f76cb4..3b5a47f 100755
--- a/meta/recipes-core/initscripts/initscripts-1.0/bootmisc.sh
+++ b/meta/recipes-core/initscripts/initscripts-1.0/bootmisc.sh
@@ -54,14 +54,7 @@  fi
 
 #
 # This is as good a place as any for a sanity check
-# /tmp should be a symlink to /var/tmp to cut down on the number
-# of mounted ramdisks.
-if test ! -L /tmp && test -d /var/tmp
-then
-	rm -rf /tmp
-	ln -sf /var/tmp /tmp
-fi
-
+#
 # Set the system clock from hardware clock
 # If the timestamp is more recent than the current time,
 # use the timestamp instead.
diff --git a/meta/recipes-core/initscripts/initscripts-1.0/volatiles b/meta/recipes-core/initscripts/initscripts-1.0/volatiles
index e0741aa..f7e2ef7 100644
--- a/meta/recipes-core/initscripts/initscripts-1.0/volatiles
+++ b/meta/recipes-core/initscripts/initscripts-1.0/volatiles
@@ -31,6 +31,7 @@  l root root 1777 /var/lock /var/volatile/lock
 l root root 0755 /var/log /var/volatile/log
 l root root 0755 /var/run /var/volatile/run
 l root root 1777 /var/tmp /var/volatile/tmp
+l root root 1777 /tmp /var/tmp
 d root root 0755 /var/lock/subsys none
 f root root 0664 /var/log/wtmp none
 f root root 0664 /var/run/utmp none