Re: [Libguestfs] [PATCH v2] parse_libvirt_xml: look for manual firmware in "/domain/os/loader"

On Thu, Dec 15, 2022 at 08:11:15PM +0200, Andrey Drobyshev wrote: > According to [1], there're different ways to specify which firmware is > to be used by a libvirt-driven VM. Namely, there's an automatic > firmware selection, e.g.: > > ... > <os firmware='(bios|efi)'> > ... > > and a manual one, e.g.: > > ... > <os> > <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader> > ... > </os> > ... > > with the latter being a way to specify UEFI firmware. So let's add this > search path as well when parsing source VM's libvirt xml. > > [1] https://libvirt.org/formatdomain.html#bios-bootloader > > Signed-off-by: Andrey Drobyshev <andrey.drobyshev(a)virtuozzo.com&gt; > Originally-by: Denis Plotnikov <dplotnikov(a)virtuozzo.com&gt; > --- > input/parse_libvirt_xml.ml | 21 +++++++++++++++++++-- > 1 file changed, 19 insertions(+), 2 deletions(-) > > diff --git a/input/parse_libvirt_xml.ml b/input/parse_libvirt_xml.ml > index 1e98ce1a..0fecde33 100644 > --- a/input/parse_libvirt_xml.ml > +++ b/input/parse_libvirt_xml.ml > @@ -27,6 +27,9 @@ open Xpath_helpers > open Types > open Utils > > +(* Detect that "/domain/os/loader" node contains path to UEFI firmware. *) > +let loader_contains_ovmf_re = PCRE.compile "/OVMF_CODE" > + > type disk = { > d_format : string option; (* Disk format from XML if known. *) > d_type : disk_type; (* Disk type and extra information. *) > @@ -446,12 +449,26 @@ let parse_libvirt_xml ?conn xml = > done; > List.rev !nics in > > - (* Firmware. *) > + (* Firmware. > + * If "/domain/os" node doesn't contain "firmware" attribute (automatic > + * firmware), we look for the presence of "OVMF_CODE" in "/domain/os/loader" > + * node (manual firmware). > + * See https://libvirt.org/formatdomain.html#bios-bootloader > + *) > let firmware = > match xpath_string "/domain/os/@firmware" with > | Some "bios" -> BIOS > | Some "efi" -> UEFI > - | None | Some _ -> UnknownFirmware in > + | None | Some _ -> ( > + let loader = xpath_string "/domain/os/loader" in > + match loader with > + | None -> UnknownFirmware > + | _ -> ( > + let loader = Option.default "" loader in > + if PCRE.matches loader_contains_ovmf_re loader then UEFI > + else UnknownFirmware > + ) > + ) in > > (* Check for hostdev devices. (RHBZ#1472719) *) > let () = It seems fine to me, just want to see what Laszlo thinks first. Rich.