* intrigeri [Thu Jul 28, 2011 at 11:43:13AM +0200]:
> I quickly reviewed most of your patches.
Great!
[Just commenting on the ones that need clarification.]
> Michael Prokop wrote (26 Jul 2011 21:32:27 GMT) :
> > - 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/
> > + 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.
> > 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).
> > 25_support_lvm_for_live-media.patch
> + if [ -x /scripts/local-top/lvm2 ] ; then
> + if [ -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.
> > 30_support_multiarch_dns.patch
> > -copy_exec /lib/libnss_dns.so.* /lib # DNS server
> > +# DNS server:
> > +if ls /lib/libnss_dns.so.* >/dev/null 2>&1 ; then # non-multiarch libc
> > + copy_exec /lib/libnss_dns.so.* /lib
> > +elif ls /lib/*/libnss_dns.so.* >/dev/null 2>&1 ; then # multiarch libc
> > + for libnss in /lib/*/libnss_dns.so.* ; do
> > + copy_exec "$libnss"
> Just curious: wouldn't "copy_exec /lib/*/libnss_dns.so.*" work?
> Otherwise, this bugfix sounds most welcome to me.
No, since the signature of copy_exec is:
$1 = file to copy to ramdisk
$2 (optional) Name for the file on the ramdisk
regards,
-mika-
--
http://michael-prokop.at/ || http://adminzen.org/
http://grml-solutions.com/ || http://grml.org/
Attachment:
signature.asc
Description: Digital signature