Libguestfs - Dec 2022 - [PATCH] 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 | 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
+        )
+    ) in
 
   (* Check for hostdev devices. (RHBZ#1472719) *)
   let () -- 
2.31.1

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.drobyshev at virtuozzo.com>
> Originally-by: Denis Plotnikov <dplotnikov at 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
> +        )
> +    ) in
>  
>    (* Check for hostdev devices. (RHBZ#1472719) *)
>    let () 
Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org