On 12/16/22 20:01, 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 | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/input/parse_libvirt_xml.ml b/input/parse_libvirt_xml.ml
> index 1e98ce1a..449e3466 100644
> --- a/input/parse_libvirt_xml.ml
> +++ b/input/parse_libvirt_xml.ml
> @@ -446,12 +446,28 @@ 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 "pflash" in
> + * "/domain/os/loader/@type" attribute (manual firmware), with the latter
> + * indicating the UEFI 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 _ -> (
I'd prefer if we kept assigning UnknownFirmware to the "Some _" pattern
here. That would indicate <os firmware='something_unknown'>, so I think
we should give up further domain XML-based checks in that case.
> + let loader = xpath_string "/domain/os/loader/@type" in
> + match loader with
> + | None -> UnknownFirmware
> + | _ -> (
> + let loader = Option.default "" loader in
> + match loader with
> + | "pflash" -> UEFI
> + | _ -> UnknownFirmware
> + )
> + ) in
This looks needlessly complicated. "Option.default" is just a shorthand
for pattern matching [common/mlstdutils/std_utils.ml]:
> module Option = struct
> [...]
> let default def = function
> | None -> def
> | Some x -> x
> end
(where "function" is again syntactic sugar; it means:
> module Option = struct
> [...]
> let default def opt = match opt with
> | None -> def
> | Some x -> x
> end
)
so once you're matching patterns already, "Option.default" is
superfluous. How about this:
diff --git a/input/parse_libvirt_xml.ml b/input/parse_libvirt_xml.ml
index 1e98ce1a8694..a98cdfb76109 100644
--- a/input/parse_libvirt_xml.ml
+++ b/input/parse_libvirt_xml.ml
@@ -451,7 +451,12 @@ let parse_libvirt_xml ?conn xml =
match xpath_string "/domain/os/@firmware" with
| Some "bios" -> BIOS
| Some "efi" -> UEFI
- | None | Some _ -> UnknownFirmware in
+ | Some _ -> UnknownFirmware
+ | None -> (
+ match xpath_string "/domain/os/loader/@type" with
+ | Some "pflash" -> UEFI
+ | _ -> UnknownFirmware
+ ) in
(* Check for hostdev devices. (RHBZ#1472719) *)
let () =
Laszlo
>
> (* Check for hostdev devices. (RHBZ#1472719) *)
> let () =
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
