On Thu, Feb 16, 2012 at 02:26:57PM +0100, Jonas Meurer wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hey August,
>
> thanks for commenting again on this bugreport. See my comments below.
>
> Am 16.02.2012 13:14, schrieb Agustin Martin:
> > I have later extended the test in the lilo booted box to be able to
> > boot to two different partitions on lvm, one with testing (where
> > all kernels are installed and images created, although modules are
> > put in the shared partition) and one running stable. With my
> > patched cryptsetup I can boot from both.
> ...
> > By the way, patch still appplies cleanly to 1.4 and seems to work
> > properly, both in my lilo and grub booted boxes.
>
> That reads like your patch is well tested.
Hi again, Jonas, thanks for the quick reply
> questions, and need to do further testing with other unusual setups
> before applying it.
I tested it in my particular unusual setup, that is indeed not very general.
> Maybe you can help here.
Happy to try helping.
> This is your patch:
>
> > diff --git a/debian/initramfs/cryptroot-script
> b/debian/initramfs/cryptroot-script
> > index f83e52e..9de28a4 100644 ---
> > a/debian/initramfs/cryptroot-script +++
> > b/debian/initramfs/cryptroot-script @@ -326,7 +326,7 @@
> > setup_mapping() return 1 fi
> >
> > - NEWROOT="/dev/mapper/$cryptlvm" +
> > NEWROOT=${cmdline_root=/dev/mapper/$cryptlvm}
>
> In my tests, this didn't work as expected. For some reason, $cryptlvm
> is ignored within the specified default value for NEWROOT.
Not sure if related to the above, but I should have used
NEWROOT=${cmdline_root:-/dev/mapper/$cryptlvm}
Not using ":" might cause portability issues, and there is no need to change
$cmdline_root value so ":-" instead of "=" seems better.
Do not have the lilo booted box here to test and remember the details. I
however added some debugging lines just after that definition,
message "cmdline_root: $cmdline_root"
message "cryptlvm: $cryptlvm"
message "NEWROOT: $NEWROOT"
and values seem correct in my grub booted box. Need to test more carefully.
> > @@ -354,18 +354,35 @@ setup_mapping() #
> >
> > # Do we have any kernel boot arguments? -found=''
> > +cmdline_cryptopts='' +unset cmdline_root for opt in $(cat
> > /proc/cmdline); do case $opt in cryptopts=*) - found=yes -
> > setup_mapping "${opt#cryptopts=}" +
> > opt="${opt#cryptopts=}" + if [ -n "$opt" ]; then +
> > if [ -n
> > "$cmdline_cryptopts" ]; then +
> > cmdline_cryptopts="$cmdline_cryptopts,$opt" + else +
> > cmdline_cryptopts="$opt" + fi + fi ;; +
> > root=*) +
> > opt="${opt#root=}" + case $opt in + /*) #
> > Absolute path
> > given. Not lilo major/minor number. +
> > cmdline_root=$opt + ;; +
> > *) # lilo major/minor number (See #398957). Ignore + esac +
> > ;; esac done
> >
> > -if [ -n "$found" ]; then - exit 0 +if [ -n "$cmdline_cryptopts" ];
> > then + setup_mapping "$cmdline_cryptopts" + exit 0 fi
> >
> > # Do we have any settings from the /conf/conf.d/cryptroot file?
>
> This will break systems with more than one cryptopts-parameter.
> Currently, it should work to give several cryptopts-parameters as boot
> parameters and that way unlock more than one cryptdisk within the
> initramfs. your patch will give all cryptopts parameters as one
> argument seperated by commas to setup_mapping at once, instead of
> invoking setup_mapping for every single cryptopts-parameter. Do you
> get my point? I guess it would be enough to change the seperator from
> comma to space and do something like 'for cryptopt in
> $cmdline_cryptopts; do setup_mapping "$cryptopt"; done' later.
You are right. I have tested using
if [ -n "$cmdline_cryptopts" ]; then
# Call setup_mapping separately for each possible cryptopts= setting
for cryptopt in $cmdline_cryptopts; do
setup_mapping "$cryptopt"
done
exit 0
fi
as you propose and seems to work well in my simple case. Not tested with
multiple cryptopts= calls.
I am attaching a diff with current status of my changes. I hope to test in a
lilo booted box in some days. Will let you know.
Regards,
--
Agustin
diff --git a/debian/initramfs/cryptroot-script b/debian/initramfs/cryptroot-script
index 8c40af5..af0e772 100644
--- a/debian/initramfs/cryptroot-script
+++ b/debian/initramfs/cryptroot-script
@@ -303,7 +303,7 @@ setup_mapping()
return 1
fi
- NEWROOT="/dev/mapper/$cryptlvm"
+ NEWROOT=${cmdline_root:-/dev/mapper/$cryptlvm}
if [ "$cryptrootdev" = "yes" ]; then
# required for lilo to find the root device
echo "ROOT=$NEWROOT" >> /conf/param.conf
@@ -336,18 +336,38 @@ setup_mapping()
#
# Do we have any kernel boot arguments?
-found=''
+cmdline_cryptopts=''
+unset cmdline_root
for opt in $(cat /proc/cmdline); do
case $opt in
cryptopts=*)
- found=yes
- setup_mapping "${opt#cryptopts=}"
+ opt="${opt#cryptopts=}"
+ if [ -n "$opt" ]; then
+ if [ -n "$cmdline_cryptopts" ]; then
+ cmdline_cryptopts="$cmdline_cryptopts $opt"
+ else
+ cmdline_cryptopts="$opt"
+ fi
+ fi
;;
+ root=*)
+ opt="${opt#root=}"
+ case $opt in
+ /*) # Absolute path given. Not lilo major/minor number.
+ cmdline_root=$opt
+ ;;
+ *) # lilo major/minor number (See #398957). Ignore
+ esac
+ ;;
esac
done
-if [ -n "$found" ]; then
- exit 0
+if [ -n "$cmdline_cryptopts" ]; then
+ # Call setup_mapping separately for each possible cryptopts= setting
+ for cryptopt in $cmdline_cryptopts; do
+ setup_mapping "$cryptopt"
+ done
+ exit 0
fi
# Do we have any settings from the /conf/conf.d/cryptroot file?
