Bug 336723 – baselayout-1_12 (rev. 3163): we cannot rely on stat (associated with #291916)

• Product: Gentoo Linux
• Status: RESOLVED FIXED

[Description Amadeusz Żołnowski (RETIRED) 2010-09-10 14:22:30 UTC]

To solve bug #291916, /usr/bin/stat is used (to check if given directory is a mount point). Although we cannot rely on /usr/bin/stat at this very beginning of rc.

Reproducible: Always

Steps to Reproduce:
1. Move /usr to separate partition and append "/dev/foo /usr fs opts x y" to /etc/fstab.
2. Reboot

Actual Results:  
Fails just before mounting /proc.


Expected Results:  
Mount /proc, /sys (or skip if already mounted) and continue booting.

[Comment 1 Amadeusz Żołnowski (RETIRED) 2010-09-10 14:34:06 UTC]

Created attachment 246715 [details, diff]
functions.sh-is_mounted-is_clean.patch

Provides functions 'is_mounted' and 'is_clean'.

'is_mounted' checks if specified device is really mounted under specified mount point.

'is_clean' checks if there are any files inside given directory (omits hidden ones).

[Comment 2 Amadeusz Żołnowski (RETIRED) 2010-09-10 14:46:10 UTC]

Created attachment 246716 [details, diff]
rc-is_mounted-instead-of-stat.patch

We use new 'is_mounted' (see: functions.sh-is_mounted-is_clean.patch) instead of 'stat MOUNT_POINT -c %d' and new more generic 'is_clean' instead of check for individual files.

[Comment 3 SpanKY 2010-09-18 09:03:21 UTC]

why does is_mounted need to check /proc/mounts ?  seems like simply replacing the call to `stat` with `mountpoint` would be sufficient.

assuming /proc/mounts needs to be checked, it should be done in one shot instead of reading it line by line:
[[ $(</proc/mounts) =~ ... ]]

the is_clean needs to declare "f" local, 

[Comment 4 Amadeusz Żołnowski (RETIRED) 2010-09-18 16:31:57 UTC]

Created attachment 247871 [details, diff]
functions.sh-is_mounted-is_clean-corrected.patch

I've corrected it according to SpanKY's suggestions. Thank you!

Why I'm checking /proc/mounts? Previously there was check for files specific to particular file system, so I've done more universal check. As you can see it's optional, so invoking 'is_mounted /some/mount/point' just checks if it's a mount point.

[Comment 5 SpanKY 2010-09-19 22:17:50 UTC]

i meant, why is "is_mounted" checking /proc/mounts ?  why cant the func merely call `mountpoint` ?
is_mounted() { mountpoint -q "$1" ; }

[Comment 6 Amadeusz Żołnowski (RETIRED) 2010-09-20 11:21:20 UTC]

"is_mounted" is checking /proc/mounts if expected device is really mounted under that mount point and not something else. (Although I can imagine case when that check doesn't give full certainty.)  I agree that just calling `mountpoint` would be sufficient.  Checking /proc/mounts is paranoid a bit.  I'll remove that unnecessary part from "is_mounted" then, OK?

[Comment 7 SpanKY 2010-09-20 20:45:33 UTC]

yes, let's keep it simple for now.  it seems to have worked so far for us, so let's avoid the paranoia until we have an actual use case.

[Comment 8 Amadeusz Żołnowski (RETIRED) 2010-09-22 17:44:34 UTC]

Created attachment 248375 [details, diff]
functions.sh-is_mounted-is_clean-simplified.patch

Removed /proc/mounts check.

[Comment 9 Amadeusz Żołnowski (RETIRED) 2010-09-22 17:46:56 UTC]

Created attachment 248377 [details, diff]
rc-is_mounted-instead-of-stat-simplified.patch

Updated to changes in 'functions.sh*' patch.

[Comment 10 SpanKY 2010-09-26 22:05:13 UTC]

thanks, ive committed that

http://sources.gentoo.org/baselayout/branches/baselayout-1_12/sbin/functions.sh?r1=3176&r2=3177
http://sources.gentoo.org/baselayout/branches/baselayout-1_12/sbin/rc?r1=3176&r2=3177