On 12/16/22 19:26, Andrey Drobyshev wrote: > On 12/15/22 21:25, Richard W.M. Jones wrote: >> 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.drobys...@virtuozzo.com> >>> Originally-by: Denis Plotnikov <dplotni...@virtuozzo.com> >>> --- >>> 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. >> > > BTW speaking of the regexps, I think we might as well not use them at > all (either PCRE or any other kind). Libvirt wouldn't allow any > modifications (letter case, spaces etc.) for the attribute value, > meaning that if the attribute is present, it MUST be exactly "pflash". > So in this case I'd go with straighforward string pattern matching. >
Sounds good to me. _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
