On 07/26/2011 11:32 PM, Michael Prokop wrote:
> 01_fix_output_file.patch
applied, thanks.
> 07_support_findiso.patch
misses manpage entry, and please do the silencing of the umount call in
a seperate commit/patch, so that it can (if needed) reverted
independently from the fromiso thing.
> 10_validateroot.patch
intrigeri commented about that already.
> 16_nodhcp.patch
who uses NODHCPHOSTNAME? i couldn't find any reference in live-boot nor
initramfs-tools that makes use of this variable.
> 19_revert_toram_bootoption_to_expected_behaviour.patch
since we have d-i with l-i by default on the images these days, we
should by default copy the whole image to ram, and, if too few space
available, either automatically fall back to copying the filesystem only
(plus giving a message about it), or, failing with the advice to use a
parameter to specify that the filesystem should be copied only.
however, just 'reverting' the command without above doesn't make sense.
> 25_support_lvm_for_live-media.patch
reformated to match the documented codying style, otherwise applied, thanks.
> 27_support_static_ip.patch
looks rather intrusive, in particular, has broken formating, and i
wonder if there's no code in intramfs-tools already that can be sourced
for that.
> 30_support_multiarch_dns.patch
i'll wait for that on comments from Michal.
--
Address: Daniel Baumann, Donnerbuehlweg 3, CH-3012 Bern
Email: daniel.baumann(a)progress-technologies.net
Internet: http://people.progress-technologies.net/~daniel.baumann/
* 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/
On 07/26/2011 11:32 PM, Michael Prokop wrote:
> once again a patch bomb by the Grml project.
oh, and in general.. i'd appreciate if you could update patches so that
they have a proper name of the author in it ($Firstname $Lastname
<$email>), plus description (short one line, multiple additional lines
where needed), and formatted according to
http://live.debian.net/manual/en/html/coding-style.html.
--
Address: Daniel Baumann, Donnerbuehlweg 3, CH-3012 Bern
Email: daniel.baumann(a)progress-technologies.net
Internet: http://people.progress-technologies.net/~daniel.baumann/
On 07/28/2011 03:03 PM, Ulrich Dangel wrote:
> The main question is also do we really want to support all these
> scripts?
if possible, yes.
> I really don't think it is wortwhile to be able to use findiso
> for example on a crypted disk/nbd.
live-boot should support as many options to boot, in any possible
combination (which currently is rather messy as things are not properly
factored out yet, but.. that was speaking about the general case).
--
Address: Daniel Baumann, Donnerbuehlweg 3, CH-3012 Bern
Email: daniel.baumann(a)progress-technologies.net
Internet: http://people.progress-technologies.net/~daniel.baumann/
