Hi,
Michael Prokop wrote (28 Jul 2011 12:01:21 GMT) :
- intrigeri [Thu Jul 28, 2011 at 11:43:13AM +0200]:
umount ${mountpoint}
umount ${mountpoint} 2>/dev/null
Rationale for this seemingly unrelated change? I fear it could make debugging harder.
It's to not show an error message if live/image has been already unmounted before. For the context of the code see http://paste.pocoo.org/show/448076/
Fair enough => ack.
grep -q /live/findiso /proc/mounts && umount /root/live/findiso
The grep command could be a bit more specific, and hence more robust I believe. Isn't it possible to use the mountpoint command here?
This would mean that we'd have to add mountpoint(1) to the initramfs using copy_exec, hmmm.
/bin/mountpoint is 6,6K on my Wheezy system. According to ldd, the only lib it depends on is libc, which is probably already in the initramfs. So I don't think it's a big deal.
Else, "awk '{print $2}' < /proc/mounts | ..." could be used, but frankly, I'm not sure adding more text parsing to the mix would add any robustness at all.
=> I'd better add a few kB to the initramfs and get clean, robust and maintainable code.
10_validateroot.patch
I like defensive programming, but I fear this one might serve to hide a problem you encountered with scripts/live not dealing with some kind of particular case. Is this live-bottom script here just in case, or...?
If live-boot finds a "wrong" file system that looks OK for live-boot then the error message can be pretty confusing. Though I don't like the check for /sbin/init neither, so feel free to ignore this patch (and we'd investigate on that further).
Ok. I see the need for it, and I may see why you don't like it. Is there any file any system running live-boot has, and any other system hasn't?
25_support_lvm_for_live-media.patch
if [ -x /scripts/local-top/lvm2 ] ; thenif [ -x /scripts/local-top/mdadm ] ; then
I'm doubtful about adding support for (if I understand clearly) GRML-specific local-* scripts that are not shipped in live-boot.
They aren't Grml specific local scripts, they are shipped by the original lvm2 and mdadm Debian packages.
Oops, sorry. I'll look at this one again then.
Unless someone else has merged, in the meantime, the patches we've reached an agreement upon, I'll do it myself in a few weeks. Feel free to ping me if I seem to forget.
Bye,