On 12/15/22 19:34, Richard W.M. Jones wrote: > On Thu, Dec 15, 2022 at 06:45:46PM +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.drobys...@virtuozzo.com> >> Originally-by: Denis Plotnikov <dplotni...@virtuozzo.com> > > Laszlo, any comments on this? > >> input/parse_libvirt_xml.ml | 19 +++++++++++++++++-- >> 1 file changed, 17 insertions(+), 2 deletions(-) >> >> diff --git a/input/parse_libvirt_xml.ml b/input/parse_libvirt_xml.ml >> index 1e98ce1a..afe3fbc2 100644 >> --- a/input/parse_libvirt_xml.ml >> +++ b/input/parse_libvirt_xml.ml >> @@ -446,12 +446,27 @@ 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 re = Str.regexp_string "OVMF_CODE" in >> + let loader = Option.default "" loader in >> + try ignore (Str.search_forward re loader 0); UEFI >> + with Not_found -> UnknownFirmware > > It's a bit nicer to use PCRE regexps here, and that allows you to make > them more precise, and also to hoist the compilation to the top of the > file. You could write something like: > > (near the top of the file) > > let re_loader_contains_ovmf = PCRE.compile "/OVMF_CODE" > > (here) > > let loader = Option.default "" loader in > if PCRE.matches re_loader_contains_ovmf loader then UEFI > else UnknownFirmware >
This surely looks nicer, thanks for your note. Will add this to v2. >> + ) >> + ) in >> >> (* Check for hostdev devices. (RHBZ#1472719) *) >> let () = > > Rich. > _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
