[elbe-devel] [PATCH] hdimg: make sure that the correct partition ends up in menu.lst

[Bastian Germann]

Am 20.04.21 um 21:29 schrieb Bastian Germann:
> Am 13.04.21 um 14:34 schrieb Christian Teklenborg:
>> The grub-legacy installer loops trough the fstab entries and uses the
>> last entry to modify menu.lst. This results in always booting from the
>> last partition. Ensure that the entry with mountpoint "/boot" ends up in
>> menu.lst.
>>
>> This closes issue #296.
>>
>> Signed-off-by: Christian Teklenborg <chris at linutronix.de>
>> ---
>>   elbepack/hdimg.py | 13 +++++++++++--
>>   1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/elbepack/hdimg.py b/elbepack/hdimg.py
>> index bfcbca69..f1a72c78 100644
>> --- a/elbepack/hdimg.py
>> +++ b/elbepack/hdimg.py
>> @@ -244,10 +244,19 @@ class grubinstaller97(grubinstaller_base):
>>               do('cp -a %s %s' % (loopdev, poopdev))
>>               do('kpartx -as %s' % poopdev)
>> +            bootentry = 0
>> +
>>               for entry in self.fs.depthlist():
>> +                if entry.mountpoint.startswith("/boot"):
>> +                    bootentry_label = entry.label
>> +                    bootentry = int(entry.partnum)
>>                   do('mount /dev/mapper/poop%sp%d %s' %
>>                      (loopnum, entry.partnum, imagemntfs.fname(entry.mountpoint)))
>> +            if not bootentry:
>> +                bootentry_label = entry.label
>> +                bootentry = int(entry.partnum)
>> +
> 
> This duplicate code falls back on the loop's last entry. Please remove this and change the loop to 
> execute the two variable assignments unconditionally and at the end break on 
> entry.mountpoint.startswith("/boot").

No, just keep it as-is. Otherwise the rest of the mounts are not there.

Reviewed-by: Bastian Germann <bage at linutronix.de>

> 
>>               do("mount --bind /dev %s" % imagemntfs.fname("dev"))
>>               do("mount --bind /proc %s" % imagemntfs.fname("proc"))
>>               do("mount --bind /sys %s" % imagemntfs.fname("sys"))
>> @@ -267,9 +276,9 @@ class grubinstaller97(grubinstaller_base):
>>               # variable 'entry' (undefined-loop-variable).  entry is
>>               # defined in the previous for-loop.
>>               do(r'chroot %s sed -in "s/^# groot=.*$/# groot=\(hd0,%d\)/" %s' %
>> -               (imagemnt, int(entry.partnum) - 1, "/boot/grub/menu.lst"))
>> +               (imagemnt, bootentry - 1, "/boot/grub/menu.lst"))
>>               do(r'chroot %s sed -in "s/^# kopt=.*$/# kopt=root=LABEL=%s/" %s' %
>> -               (imagemnt, entry.label, "/boot/grub/menu.lst"))
>> +               (imagemnt, bootentry_label, "/boot/grub/menu.lst"))
>>               chroot(imagemnt, "update-grub")
>>