[Libguestfs] [v2v PATCH 3/9] create_libvirt_xml: normalize match on (s_cpu_vendor, s_cpu_model)

[Richard W.M. Jones]

On Wed, Apr 20, 2022 at 06:23:27PM +0200, Laszlo Ersek wrote:
> In the match on (s_cpu_vendor, s_cpu_model), the following expression is
> duplicated:
> 
>   e "model" ["fallback", "allow"] [PCData model]
> 
> For eliminating this, normalize the match: match each component in
> separation, in the proper order, creating a tree-like match rather than a
> table-like one.
> 
> This is done in preparation for a subsequent patch, which would otherwise
> duplicate even more code.
> 
> We preserve the behavior added by commit 2a576b7cc5c3 ("v2v: -o libvirt:
> Don't write only <vendor> without <model> (RHBZ#1591789).", 2018-06-21);
> the change only simplifies the code.
> 
> With the elimination of the table-like pattern matching, we need not
> specifically mention the "CPU vendor specified without CPU model" libvirt
> error.
> 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2076013
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---
>  output/create_libvirt_xml.ml | 22 ++++++++------------
>  1 file changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/output/create_libvirt_xml.ml b/output/create_libvirt_xml.ml
> index 4e5bbceffabd..d2ca894d60bb 100644
> --- a/output/create_libvirt_xml.ml
> +++ b/output/create_libvirt_xml.ml
> @@ -161,65 +161,61 @@ let create_libvirt_xml ?pool source inspect
>    );
>  
>  
>    (match get_osinfo_id inspect with
>     | None -> ()
>     | Some osinfo_id ->
>       List.push_back_list body [
>         e "metadata" [] [
>           e "libosinfo:libosinfo" ["xmlns:libosinfo", "http://libosinfo.org/xmlns/libvirt/domain/1.0"] [
>             e "libosinfo:os" ["id", osinfo_id] [];
>           ];
>         ];
>       ];
>    );
>  
>    let memory_k = source.s_memory /^ 1024L in
>    List.push_back_list body [
>      e "memory" ["unit", "KiB"] [PCData (Int64.to_string memory_k)];
>      e "currentMemory" ["unit", "KiB"] [PCData (Int64.to_string memory_k)];
>      e "vcpu" [] [PCData (string_of_int source.s_vcpu)]
>    ];
>  
>    if source.s_cpu_vendor <> None || source.s_cpu_model <> None ||
>       source.s_cpu_topology <> None then (
>      let cpu = ref [] in
>  
> -    (match source.s_cpu_vendor, source.s_cpu_model with
> -     | _, None ->
> -         (* This also avoids the libvirt error:
> -          * "CPU vendor specified without CPU model".
> -          *)
> -         ()
> -     | None, Some model ->
> -        List.push_back cpu (e "model" ["fallback", "allow"] [PCData model])
> -     | Some vendor, Some model ->
> -        List.push_back_list cpu [
> -          e "vendor" [] [PCData vendor];
> -          e "model" ["fallback", "allow"] [PCData model]
> -        ]
> +    (match source.s_cpu_model with
> +     | None -> ()
> +     | Some model ->
> +         (match source.s_cpu_vendor with
> +          | None -> ()
> +          | Some vendor ->
> +              List.push_back cpu (e "vendor" [] [PCData vendor])
> +         );
> +         List.push_back cpu (e "model" ["fallback", "allow"] [PCData model])
>      );
>      (match source.s_cpu_topology with
>       | None -> ()
>       | Some { s_cpu_sockets; s_cpu_cores; s_cpu_threads } ->
>          let topology_attrs = [
>            "sockets", string_of_int s_cpu_sockets;
>            "cores", string_of_int s_cpu_cores;
>            "threads", string_of_int s_cpu_threads;
>          ] in
>          List.push_back cpu (e "topology" topology_attrs [])
>      );
>  
>      List.push_back_list body [ e "cpu" [ "match", "minimum" ] !cpu ]
>    );
>  
>    let uefi_firmware =
>      match target_firmware with
>      | TargetBIOS -> None
>      | TargetUEFI -> Some (find_uefi_firmware guestcaps.gcaps_arch) in
>    let machine, secure_boot_required =
>      match guestcaps.gcaps_machine, uefi_firmware with
>      | _, Some { Uefi.flags = flags }
>           when List.mem Uefi.UEFI_FLAG_SECURE_BOOT_REQUIRED flags ->
>         (* Force machine type to Q35 because PC does not support
>          * secure boot.  We must remove this when we get the
>          * correct machine type from libosinfo in future. XXX
> -- 

Reviewed-by: Richard W.M. Jones <rjones at redhat.com>

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