On 11/2/23 12:23, Richard W.M. Jones wrote:
On Tue, Oct 31, 2023 at 09:45:15PM +0100, Laszlo Ersek wrote:
> On 10/31/23 14:17, Andrey Drobyshev wrote:
>> On some older Debian-based distros (in particular, this is true for
>> Debian 8) GRUB config file doesn't reside on EFI partition, but rather
>> at /boot/grub/grub.cfg even if UEFI firmware is being used. As a
>> result, in the UEFI case we get "error: no bootloader detected".
Let's
>> just append this path to the end of the corresponding list as a workaround
>> when dealing with UEFI guests.
>>
>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev(a)virtuozzo.com>
>> ---
>> mldrivers/linux_bootloaders.ml | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/mldrivers/linux_bootloaders.ml b/mldrivers/linux_bootloaders.ml
>> index 6f8857e..91c5ab9 100644
>> --- a/mldrivers/linux_bootloaders.ml
>> +++ b/mldrivers/linux_bootloaders.ml
>> @@ -375,6 +375,18 @@ let detect_bootloader (g : G.guestfs) root i_firmware =
>> with G.Error msg ->
>> error (f_"could not find bootloader mount point (%s): %s") mp
msg in
>>
>> + (*
>> + * Workaround for older UEFI-based Debian which may not have
>> + * /boot/efi/EFI/debian/grub.cfg.
>> + *)
>> + let paths =
>> + if g#exists "/boot/grub/grub.cfg" then
>> + match i_firmware with
>> + | Firmware.I_BIOS -> paths
>> + | I_UEFI _ -> paths @ ["/boot/grub/grub.cfg"]
>> + else paths
>> + in
>> +
>> (* We can determine if the bootloader config file is grub 1 or
>> * grub 2 just by looking at the filename.
>> *)
>
> I'm not sure if the g#exists check is helpful; after all, in the
"loop"
> function, we have a stricter g#is_file check. IOW, if the special path
> does not exist (or does not resolve to a regular file, after following
> symlinks), then "loop" will drop the candidate anyway. And if g#exists
> returns true, then "loop" may still drop the candidate (or may keep it).
> So IMO, just the inner "match" would suffice.
>
> Anyway, I don't want to draw this out forever, so with the g#exists
> check kept, or removed:
>
> Reviewed-by: Laszlo Ersek <lersek(a)redhat.com>
Andrey, if we push this patch (to libguestfs-common) then we no longer
need the other patch as the i_firmware parameter is still required, is
that correct? I can push this once confirmed.
Hello Richard,
Yes, you're right, this single patch is sufficient as it doesn't change
the function signature. Feel free to omit the g#esists part as Laszlo
suggested.
BTW I apologise for the mailing list problems. We are in the
process
of moving the server, and the mailing address is now
'guestfs(a)lists.libguestfs.org' with archives at
https://lists.libguestfs.org . I haven't made the announcement yet.
Thanks for pointing out, automated response with the announcement did
help here.
Andrey