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

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 at virtuozzo.com>
Originally-by: Denis Plotnikov <dplotnikov at 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 () -- 
2.31.1

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 at virtuozzo.com>
> Originally-by: Denis Plotnikov <dplotnikov at 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.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW