[Libguestfs] Re: [COMMON PATCH v2] mldrivers: look for bootloader config in /boot/grub/grub.cfg in case of UEFI

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&gt; >>> --- >>> 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&gt; > > 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 > &#39;guestfs(a)lists.libguestfs.org&#39; 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