[Grml-devel] Grml.org patch bomb

[Michael Prokop]

* 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/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://ml.grml.org/pipermail/grml-devel/attachments/20110728/6e2caef0/attachment.pgp>