Currently, if RC_DEIVCES_TARBALL=yes, the halt.sh script will save all devices on a shutdown, which sometimes may be undesirable. These devices may include devices that are sysfs aware, but are compressed anyway. They may not exist after a restart, due to modules being loaded at shutdown, but not at boot time... etc. The reasons why this is not a good idea are obvious, otherwise no one would care about udev or devfs. I suggest a minor enhancement to the /etc/init.d/halt.sh script, that will make it only save devices that are not taken care of by udev, which is its original purpose after all. And here it is: --- halt.sh.bak 2004-07-15 12:31:50.000000000 +0900 +++ halt.sh 2004-07-15 12:42:50.682505258 +0900 @@ -22,8 +22,17 @@ then ebegin "Saving device nodes" cd /dev - try tar -jclpf "/tmp/devices-$$.tar.bz2" * + # A few extras, to *not* save sysfs-aware devices + find . -xdev -not -type d -not -type f | cut -d/ -f2- > /tmp/devices.real.$$ + # add a few of my own + echo stdin >> /tmp/devices.real.$$ + echo stdout >> /tmp/devices.real.$$ + echo stderr >> /tmp/devices.real.$$ + udevinfo -d | awk '/^N|S: ..*/ { print $2}' > /tmp/devices.udev.$$ + try tar -jclpf "/tmp/devices-$$.tar.bz2" \ + `fgrep -v -f /tmp/devices.udev.$$ < /tmp/devices.real.$$` try mv -f "/tmp/devices-$$.tar.bz2" /lib/udev-state/devices.tar.bz2 + try rm -f /tmp/devices.{real,udev}.$$ eend 0 fi
Ah, I didn't think of devices with multiple symlinks. Consider the following change to the awk line: - udevinfo -d | awk '/^N|S: ..*/ { print $2}' > /tmp/devices.udev.$$ + udevinfo -d | awk '/^N|S: ..*/ { i=1; while (i++ < NF) { print $i }}' > /tmp/devices.udev.$$
Created attachment 35451 [details, diff] halt-udev-tarball.patch I decided to apply the suggested on a few more of my machines, so I ended up with a proper patch and an ebuild in my overlay. This file is the one that goes in $PORTDIR/sys-apps/baselayout/files/ It fixes something I had overlooked with the suggestions above, so it now properly makes for the compression of less files. It now only tries to find block devs, character devs, or symlinks.
Created attachment 35452 [details, diff] baselayout.ebuild.patch A combined patch for the newest stable x86 and ~x86 baselayout ebuilds (1.9.4-r3 and 1.10.1-r2).
Created attachment 35617 [details, diff] halt-udev-tarball.patch There is no action on the bug, but I'll keep posting, and posting, and posting :) The fgrep command needs the "-x" option, to grep things properly. Including a modified patch.
might as well mention that i strongly dislike our current tarball hack, and something similar to this is, IMHO, a bit better.
Adding myself, since I think this is a bit interesting... =]
This looks good to me. I'll get it in soon, currently working on a couple other bugs.
I do not see why you need this bit: ---- + cat <<EOF >>/tmp/devices.udev.$$ +stdin +stdout +stderr +core +fd +initctl +EOF ---- As /sbin/rc already talkes care of that ... ?
Also, I think your comments is misleading - something more like this: ---- Index: init.d/halt.sh =================================================================== RCS file: /var/cvsroot/gentoo-src/rc-scripts/init.d/halt.sh,v retrieving revision 1.50 diff -u -r1.50 halt.sh --- init.d/halt.sh 21 Apr 2004 17:09:18 -0000 1.50 +++ init.d/halt.sh 22 Jul 2004 18:59:31 -0000 @@ -22,8 +22,17 @@ then ebegin "Saving device nodes" cd /dev - try tar -jclpf "/tmp/devices-$$.tar.bz2" * + # Find all devices + find . -xdev -type b -or -type c -or -type l | cut -d/ -f2- > \ + /tmp/devices.real.$$ + # Figure out what udev created + udevinfo -d | awk '/^N|S: ..*/ { i=1; while (i++<NF) { print $i}}' > \ + /tmp/devices.udev.$$ + # Now only tarball those not created by udev + try tar -jclpf "/tmp/devices-$$.tar.bz2" \ + `fgrep -x -v -f /tmp/devices.udev.$$ < /tmp/devices.real.$$` try mv -f "/tmp/devices-$$.tar.bz2" /lib/udev-state/devices.tar.bz2 + try rm -f /tmp/devices.{real,udev}.$$ eend 0 fi
Bah, don't worry, I was on droids :/
Ok, rather this then with comments fixed, some 80+ liner fixes and without the here-document (I am not a big fan, this is more compact and dont depend on cat): ---- Index: init.d/halt.sh =================================================================== RCS file: /var/cvsroot/gentoo-src/rc-scripts/init.d/halt.sh,v retrieving revision 1.50 diff -u -r1.50 halt.sh --- init.d/halt.sh 21 Apr 2004 17:09:18 -0000 1.50 +++ init.d/halt.sh 22 Jul 2004 20:07:24 -0000 @@ -22,8 +22,22 @@ then ebegin "Saving device nodes" cd /dev - try tar -jclpf "/tmp/devices-$$.tar.bz2" * + # Find all devices + find . -xdev -type b -or -type c -or -type l | cut -d/ -f2- > \ + /tmp/devices.real.$$ + # Figure out what udev created + udevinfo -d | awk '/^N|S: ..*/ { i=1; while (i++<NF) { print $i}}' > \ + /tmp/devices.udev.$$ + # These ones we also do not want in there + for x in stdin stdout stderr core fd initctl + do + echo "${x}" >> /tmp/devices.udev.$$ + done + # Now only tarball those not created by udev + try tar -jclpf "/tmp/devices-$$.tar.bz2" \ + `fgrep -x -v -f /tmp/devices.udev.$$ < /tmp/devices.real.$$` try mv -f "/tmp/devices-$$.tar.bz2" /lib/udev-state/devices.tar.bz2 + try rm -f /tmp/devices.{real,udev}.$$ eend 0 fi
It's getting prettier by the minute. We might want to exclude MAKEDEV from the list as well. Figures one wouldn't want MAKEDEV on a udev system, right? What about sndstat? gregkh suggested something on gentoo-dev: == begin quote == > sndstat Hm, are you sure you need this? This should just work on the newer 2.6 kernels. == end quote ==
Right. Latest diff - anything else needing tweaking? --- Index: init.d/halt.sh =================================================================== RCS file: /var/cvsroot/gentoo-src/rc-scripts/init.d/halt.sh,v retrieving revision 1.50 diff -u -r1.50 halt.sh --- init.d/halt.sh 21 Apr 2004 17:09:18 -0000 1.50 +++ init.d/halt.sh 24 Jul 2004 16:47:22 -0000 @@ -22,8 +22,22 @@ then ebegin "Saving device nodes" cd /dev - try tar -jclpf "/tmp/devices-$$.tar.bz2" * + # Find all devices + find . -xdev -type b -or -type c -or -type l | cut -d/ -f2- > \ + /tmp/devices.real.$$ + # Figure out what udev created + udevinfo -d | awk '/^N|S: ..*/ { i=1; while (i++<NF) { print $i}}' > \ + /tmp/devices.udev.$$ + # These ones we also do not want in there + for x in MAKEDEV core fd initctl sndstat stderr stdin stdout + do + echo "${x}" >> /tmp/devices.udev.$$ + done + # Now only tarball those not created by udev + try tar -jclpf "/tmp/devices-$$.tar.bz2" \ + `fgrep -x -v -f /tmp/devices.udev.$$ < /tmp/devices.real.$$` try mv -f "/tmp/devices-$$.tar.bz2" /lib/udev-state/devices.tar.bz2 + try rm -f /tmp/devices.{real,udev}.$$ eend 0 fi
How about using mktemp instead of insecure temp files?
I am not sure $$ will be so easy to predict, but here is with mktemp. Somebody will need to test it though ... ---- Index: init.d/halt.sh =================================================================== RCS file: /var/cvsroot/gentoo-src/rc-scripts/init.d/halt.sh,v retrieving revision 1.50 diff -u -r1.50 halt.sh --- init.d/halt.sh 21 Apr 2004 17:09:18 -0000 1.50 +++ init.d/halt.sh 27 Jul 2004 18:20:24 -0000 @@ -20,10 +20,26 @@ eend $? elif [ ! -e /dev/.devfsd -a -e /dev/.udev -a "${RC_DEVICE_TARBALL}" = "yes" ] then + mytmp="`mktemp XXXXXX`" + ebegin "Saving device nodes" cd /dev - try tar -jclpf "/tmp/devices-$$.tar.bz2" * - try mv -f "/tmp/devices-$$.tar.bz2" /lib/udev-state/devices.tar.bz2 + # Find all devices + find . -xdev -type b -or -type c -or -type l | cut -d/ -f2- > \ + /tmp/devices.real.${mytmp} + # Figure out what udev created + udevinfo -d | awk '/^N|S: ..*/ { i=1; while (i++<NF) { print $i}}' > \ + /tmp/devices.udev.${mytmp} + # These ones we also do not want in there + for x in MAKEDEV core fd initctl sndstat stderr stdin stdout + do + echo "${x}" >> /tmp/devices.udev.${mytmp} + done + # Now only tarball those not created by udev + try tar -jclpf "/tmp/devices-${mytmp}.tar.bz2" \ + `fgrep -x -v -f /tmp/devices.udev.${mytmp} < /tmp/devices.real.${mytmp}` + try mv -f "/tmp/devices-${mytmp}.tar.bz2" /lib/udev-state/devices.tar.bz2 + try rm -f /tmp/devices.{real,udev}.${mytmp} eend 0 fi
This is not the proper usage of mktemp. mktemp not only gives you a random thingie, but also creates the file for you. So you end up with a new file in the directory you are running mktemp from. I suggest using these: myfileudev=$(mktemp /tmp/devices.udev.XXXXXX) myfilereal=$(mktemp /tmp/devices.real.XXXXXX) and then use the variables. I'll refrain from reposting the patch.
Az, you don't have to guess $$, you just plaster a range of stuff in /tmp and hope that you get lucky. Regarding mktemp, Georgi is right about usage. The only additional thing is to check to make sure that mktemp did not fail; in that case you should skip the tarring stuff. It's a remote chance but we might as well get it right while we're editing this code. There's also no check presently to make sure that tar doesn't fail. If /tmp is full (this happens with relatively high frequency on some systems) then the tar will fail, in which case it's better to keep the existing tarball than try to use a broken one.
> There's also no check presently to make sure that tar doesn't fail. Isn't that what "try" is for?
Heh, you're right. It's a little scary since anything that fails the "try" test will result in an immediate unmounting of filesystems and reboot of the machine, following the message Since this is a critical task, startup cannot continue. But oh well.
Why exactly is it not the correct usage of mktemp? Because its not how its examples show to use it?
From mktemp(1): The template may be any filename with some number of `Xs' appended to it, for example /tmp/tfile.XXXXXXXXXX ... If mktemp can successfully generate a unique filename, the file (or directory) is created with file permissions such that it is only readable and writable by its owner (unless the -u flag is given) and the filename is printed to standard output. tmp=$(mktemp /tmp/azarahs_temp_file.XXXXXXXXX) if [ -z "${tmp}" ]; then # tmpfile creation was not successful, mayday, mayday! fi
Yes, but: 1) it will always be root 2) /tmp should be writable (or at least that is the assumption on 99% systems, and I am not sure what should be used instead if not [TMPDIR,TMP,tmp,etc]) 3) if it fails, there is not much you can do anyhow, and one of the try's should catch it 4) it is much simpler So, yes, it will be more correct, but with specific application, it will not gain much except complexity ... More clear why I asked what I did?
Bah - except for the fact that it will creat a temp file whever it was run :/
Still needs testing: ---- Index: init.d/halt.sh =================================================================== RCS file: /var/cvsroot/gentoo-src/rc-scripts/init.d/halt.sh,v retrieving revision 1.50 diff -u -r1.50 halt.sh --- init.d/halt.sh 21 Apr 2004 17:09:18 -0000 1.50 +++ init.d/halt.sh 28 Jul 2004 20:18:18 -0000 @@ -21,10 +21,35 @@ elif [ ! -e /dev/.devfsd -a -e /dev/.udev -a "${RC_DEVICE_TARBALL}" = "yes" ] then ebegin "Saving device nodes" - cd /dev - try tar -jclpf "/tmp/devices-$$.tar.bz2" * - try mv -f "/tmp/devices-$$.tar.bz2" /lib/udev-state/devices.tar.bz2 - eend 0 + # Handle our temp files + devices_udev="`mktemp /tmp/devices.udev.XXXXXX`" + devices_real="`mktemp /tmp/devices.real.XXXXXX`" + device_tarball="`mktemp /tmp/devices-XXXXXX`" + + if [ -z "${devices_udev}" -o -z "${devices_real}" -o \ + -z "${device_tarball}" ] + then + eend 1 "Could not create temporary files!" + else + cd /dev + # Find all devices + find . -xdev -type b -or -type c -or -type l | cut -d/ -f2- > \ + "${devices_real}" + # Figure out what udev created + udevinfo -d | awk '/^N|S: ..*/ { i=1; while (i++<NF) { print $i}}' > \ + "${devices_udev}" + # These ones we also do not want in there + for x in MAKEDEV core fd initctl sndstat stderr stdin stdout + do + echo "${x}" >> "${devices_udev}" + done + # Now only tarball those not created by udev + try tar -jclpf "${device_tarball}" \ + `fgrep -x -v -f "${devices_udev}" < "${devices_real}"` + try mv -f "${device_tarball}" /lib/udev-state/devices.tar.bz2 + try rm -f "${devices_udev}" "${devices_real}" + eend 0 + fi fi # Try to unmount all tmpfs filesystems not in use, else a deadlock may
Re: Comment #19 > It's a little scary since anything that fails the "try" test will result in an immediate unmounting of filesystems and reboot of the machine, following the message If this is so, maybe "rm -f", "mv" etc should not be "tr[y]"ied, because anyway the system should not panic if any of those fail. Re: Comment #22, Comment #23 Also, just because mktemp managed to create "tmpfile.ABCDEF" doesn't mean that you can reuse the random suffix to use with "othertmpfile.ABCDEF". For all you know, "othertmpfile.ABCDEF" might already exist and it might be used by another process. mktemp creates a random extension but it also checks if the file doesn't already exist (pretty sure I read it in manpage somewhere, could be the glibc manpage). If you want to get away with a single mktemp: "mktemp -d /tmp/halt.XXXXXX"; then put the files in that directory. The -t option of mktemp is also interesting. mktemp -t -d halt.XXXXXX
The mktemp stuff should be ok now. For the try issue - the only other way I can think of, is maybe adding a derivative of try that do not sulogin, but prints the error, beeps and then pause for a second or two?
Or maybe do exactly the same as try, but do not unmount and reboot?
Aron, if you do not have issues, I am going to merge what we currently have. I will think about the try issue some more at a later stage ...
ok, az as committed some fixes for this please re-open if 1.11.5 doesnt work properly :)
udevinfo -d is missing from the latest udev-046. Is it gone forever or is it simply not developed for the new database format? In the former case, the fixes from this bug would have to be fixed.
