Libguestfs - Jun 2018 - Re: [PATCH] v2v: Set machine type explicitly for outputs which support it (RHBZ#1581428).

QEMU for x86 supports two machine types, "pc" (emulating the ancient
Intel i440FX chipset originally used by the Pentium Pro), and "q35"
(https://wiki.qemu.org/Features/Q35).

Currently virt-v2v does not set any machine type, so libvirt or the
target hypervisor will choose some default, probably i440fx.  Newer
versions of libvirt and QEMU will probably switch over to defaulting
to Q35 in the near future.

None of this matters for reasonably new guests since they can boot
with either chipset.  However there are some very old guests (notably
Windows XP) which cannot handle Q35.

This commit changes virt-v2v so it always tries to specify the machine
type explicitly (assuming the target supports that, and not all of
them do).  We pivot around the year 2007 which happens to be the year
that Q35 hardware was first available.  Any guest OS which was first
shipped earlier than 2007 will be given i440fx.  Any guest OS from
2007 or later will be given Q35.

For non-x86 architectures we select the "virt" model which will
probably only work for AArch64.  More work is needed for POWER.

A note about secure boot: Previous to this patch, if a guest used UEFI
and we detected that UEFI needed secure boot support, then we forced
the machine type to Q35 (which was wrong for non-x86 architectures).
In this patch I removed the code to force Q35, since any guest which
is using secure boot will be new enough that it'll be using Q35 anyway
after this patch (on x86).
---
 v2v/convert_linux.ml      | 20 ++++++++++++++++++++
 v2v/convert_windows.ml    | 10 ++++++++++
 v2v/create_libvirt_xml.ml | 21 ++++++++++++++-------
 v2v/create_ovf.ml         |  2 ++
 v2v/output_glance.ml      |  5 +++++
 v2v/output_qemu.ml        | 14 ++++++++------
 v2v/test-v2v-i-ova.xml    |  2 +-
 v2v/types.ml              |  8 ++++++++
 v2v/types.mli             |  6 ++++--
 9 files changed, 72 insertions(+), 16 deletions(-)

diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
index 02dc2fee2..4dbf2aa41 100644
--- a/v2v/convert_linux.ml
+++ b/v2v/convert_linux.ml
@@ -122,6 +122,25 @@ let convert (g : G.guestfs) inspect source output rcaps  
     SELinux_relabel.relabel g;
 
+    (* Pivot on the year 2007.  Any Linux distro from earlier than
+     * 2007 should use i440fx, anything 2007 or newer should use q35.
+     *)
+    let machine +      match inspect.i_arch, inspect.i_distro,
inspect.i_major_version with
+      | ("i386"|"x86_64"), "fedora", _ -> Q35
+      | ("i386"|"x86_64"),
("rhel"|"centos"|"scientificlinux"|
+                            "redhat-based"|"oraclelinux"),
major ->
+         if major <= 4 then I440FX else Q35
+      | ("i386"|"x86_64"),
("sles"|"suse-based"|"opensuse"), major ->
+         if major < 10 then I440FX else Q35
+      | ("i386"|"x86_64"),
("debian"|"ubuntu"|"linuxmint"|
+                            "kalilinux"), major ->
+         if major < 4 then I440FX else Q35
+
+      (* reasonable default for all modern Linux kernels *)
+      | ("i386"|"x86_64"), _, _ -> Q35
+      | _ -> Virt in
+
     (* Return guest capabilities from the convert () function. *)
     let guestcaps = {
       gcaps_block_bus = block_type;
@@ -130,6 +149,7 @@ let convert (g : G.guestfs) inspect source output rcaps     
gcaps_virtio_rng = kernel.ki_supports_virtio_rng;
       gcaps_virtio_balloon = kernel.ki_supports_virtio_balloon;
       gcaps_isa_pvpanic = kernel.ki_supports_isa_pvpanic;
+      gcaps_machine = machine;
       gcaps_arch = Utils.kvm_arch inspect.i_arch;
       gcaps_acpi = acpi;
     } in
diff --git a/v2v/convert_windows.ml b/v2v/convert_windows.ml
index 163319545..97882c377 100644
--- a/v2v/convert_windows.ml
+++ b/v2v/convert_windows.ml
@@ -212,6 +212,15 @@ let convert (g : G.guestfs) inspect source output rcaps    
warning (f_"this guest has Anti-Virus (AV) software and a new virtio block
device driver was installed.  In some circumstances, AV may prevent new drivers
from working (resulting in a 7B boot error).  If this happens, try disabling AV
before doing the conversion.");
     );
 
+    (* Pivot on the year 2007.  Any Windows version from earlier than
+     * 2007 should use i440fx, anything 2007 or newer should use q35.
+     * Luckily this coincides almost exactly with the release of NT 6.
+     *)
+    let machine +      match inspect.i_arch, inspect.i_major_version with
+      | ("i386"|"x86_64"), major -> if major < 6 then
I440FX else Q35
+      | _ -> Virt in
+
     (* Return guest capabilities from the convert () function. *)
     let guestcaps = {
       gcaps_block_bus = block_driver;
@@ -220,6 +229,7 @@ let convert (g : G.guestfs) inspect source output rcaps     
gcaps_virtio_rng = virtio_rng_supported;
       gcaps_virtio_balloon = virtio_ballon_supported;
       gcaps_isa_pvpanic = isa_pvpanic_supported;
+      gcaps_machine = machine;
       gcaps_arch = Utils.kvm_arch inspect.i_arch;
       gcaps_acpi = true;
     } in
diff --git a/v2v/create_libvirt_xml.ml b/v2v/create_libvirt_xml.ml
index fbe90eeaa..4b36ffb8e 100644
--- a/v2v/create_libvirt_xml.ml
+++ b/v2v/create_libvirt_xml.ml
@@ -86,10 +86,6 @@ let create_libvirt_xml ?pool source target_buses guestcaps
     | Some { Uefi.flags = flags }
          when List.mem Uefi.UEFI_FLAG_SECURE_BOOT_REQUIRED flags -> true
     | _ -> false in
-  (* Currently these are required by secure boot, but in theory they
-   * might be independent properties.
-   *)
-  let machine_q35 = secure_boot_required in
   let smm = secure_boot_required in
 
   (* We have the machine features of the guest when it was on the
@@ -140,7 +136,18 @@ let create_libvirt_xml ?pool source target_buses guestcaps
 
   (* The <os> section subelements. *)
   let os_section -    let machine = if machine_q35 then [ "machine",
"q35" ] else [] in
+    let os = ref [] in
+
+    let machine +      match guestcaps.gcaps_machine with
+      | I440FX -> "pc"
+      | Q35 -> "q35"
+      | Virt -> "virt" in
+
+    List.push_back os
+                   (e "type" ["arch", guestcaps.gcaps_arch;
+                              "machine", machine]
+                      [PCData "hvm"]);
 
     let loader        match uefi_firmware with
@@ -152,8 +159,8 @@ let create_libvirt_xml ?pool source target_buses guestcaps
              [ PCData code ];
            e "nvram" ["template", vars_template] [] ] in
 
-    (e "type" (["arch", guestcaps.gcaps_arch] @ machine)
[PCData "hvm"])
-    :: loader in
+    List.push_back_list os loader;
+    !os in
 
   List.push_back_list body [
     e "os" [] os_section;
diff --git a/v2v/create_ovf.ml b/v2v/create_ovf.ml
index 9e0c772fd..81357b55e 100644
--- a/v2v/create_ovf.ml
+++ b/v2v/create_ovf.ml
@@ -602,6 +602,8 @@ let rec create_ovf source targets guestcaps inspect
                                      source.s_vcpu memsize_mb)]
       ] in
 
+      (* XXX How to set machine type for Q35? *)
+
       List.push_back virtual_hardware_section_items (
         e "Item" [] ([
           e "rasd:Caption" [] [PCData (sprintf "%d virtual
cpu" source.s_vcpu)];
diff --git a/v2v/output_glance.ml b/v2v/output_glance.ml
index c334def42..96c31da59 100644
--- a/v2v/output_glance.ml
+++ b/v2v/output_glance.ml
@@ -86,6 +86,11 @@ object
         (match guestcaps.gcaps_video with
          | QXL -> "qxl"
          | Cirrus -> "cirrus");
+        "hw_machine_type",
+        (match guestcaps.gcaps_machine with
+         | I440FX -> "pc"
+         | Q35 -> "q35"
+         | Virt -> "virt");
         "architecture", guestcaps.gcaps_arch;
         "hypervisor_type", "kvm";
         "vm_mode", "hvm";
diff --git a/v2v/output_qemu.ml b/v2v/output_qemu.ml
index 952660de2..487363520 100644
--- a/v2v/output_qemu.ml
+++ b/v2v/output_qemu.ml
@@ -61,12 +61,14 @@ object
       | Some { Uefi.flags }
            when List.mem Uefi.UEFI_FLAG_SECURE_BOOT_REQUIRED flags -> true
       | _ -> false in
-    (* Currently these are required by secure boot, but in theory they
-     * might be independent properties.
-     *)
-    let machine_q35 = secure_boot_required in
     let smm = secure_boot_required in
 
+    let machine +      match guestcaps.gcaps_machine with
+      | I440FX -> "pc"
+      | Q35 -> "q35"
+      | Virt -> "virt" in
+
     (* Construct the command line.  Note that the [Qemuopts]
      * module deals with shell and qemu comma quoting.
      *)
@@ -80,8 +82,8 @@ object
 
     flag "-no-user-config"; flag "-nodefaults";
     arg "-name" source.s_name;
-    arg_list "-machine" (if machine_q35 then ["q35"] else
[] @
-                         if smm then ["smm=on"] else [] @
+    arg_list "-machine" (machine ::
+                         (if smm then ["smm=on"] else []) @
                          ["accel=kvm:tcg"]);
 
     (match uefi_firmware with
diff --git a/v2v/test-v2v-i-ova.xml b/v2v/test-v2v-i-ova.xml
index 5a303b80a..15925ce6e 100644
--- a/v2v/test-v2v-i-ova.xml
+++ b/v2v/test-v2v-i-ova.xml
@@ -10,7 +10,7 @@
     <apic/>
   </features>
   <os>
-    <type arch='x86_64'>hvm</type>
+    <type arch='x86_64' machine='q35'>hvm</type>
   </os>
   <on_poweroff>destroy</on_poweroff>
   <on_reboot>restart</on_reboot>
diff --git a/v2v/types.ml b/v2v/types.ml
index da1192ec3..a569759c7 100644
--- a/v2v/types.ml
+++ b/v2v/types.ml
@@ -398,6 +398,7 @@ type guestcaps = {
   gcaps_virtio_rng : bool;
   gcaps_virtio_balloon : bool;
   gcaps_isa_pvpanic : bool;
+  gcaps_machine : guestcaps_machine;
   gcaps_arch : string;
   gcaps_acpi : bool;
 }
@@ -409,6 +410,7 @@ and requested_guestcaps = {
 and guestcaps_block_type = Virtio_blk | Virtio_SCSI | IDE
 and guestcaps_net_type = Virtio_net | E1000 | RTL8139
 and guestcaps_video_type = QXL | Cirrus
+and guestcaps_machine = I440FX | Q35 | Virt
 
 let string_of_block_type = function
   | Virtio_blk -> "virtio-blk"
@@ -421,17 +423,23 @@ let string_of_net_type = function
 let string_of_video = function
   | QXL -> "qxl"
   | Cirrus -> "cirrus"
+let string_of_machine = function
+  | I440FX -> "i440fx"
+  | Q35 -> "q35"
+  | Virt -> "virt"
 
 let string_of_guestcaps gcaps    sprintf "\
 gcaps_block_bus = %s
 gcaps_net_bus = %s
 gcaps_video = %s
+gcaps_machine = %s
 gcaps_arch = %s
 gcaps_acpi = %b
 " (string_of_block_type gcaps.gcaps_block_bus)
   (string_of_net_type gcaps.gcaps_net_bus)
   (string_of_video gcaps.gcaps_video)
+  (string_of_machine gcaps.gcaps_machine)
   gcaps.gcaps_arch
   gcaps.gcaps_acpi
 
diff --git a/v2v/types.mli b/v2v/types.mli
index 291707b74..3eb946886 100644
--- a/v2v/types.mli
+++ b/v2v/types.mli
@@ -240,8 +240,9 @@ type guestcaps = {
   gcaps_virtio_balloon : bool;  (** Guest supports virtio balloon. *)
   gcaps_isa_pvpanic : bool;     (** Guest supports ISA pvpanic device. *)
 
-  gcaps_arch : string;      (** Architecture that KVM must emulate. *)
-  gcaps_acpi : bool;        (** True if guest supports acpi. *)
+  gcaps_machine : guestcaps_machine; (** Machine model. *)
+  gcaps_arch : string;          (** Architecture that KVM must emulate. *)
+  gcaps_acpi : bool;            (** True if guest supports acpi. *)
 }
 (** Guest capabilities after conversion.  eg. Was virtio found or installed? *)
 
@@ -257,6 +258,7 @@ and requested_guestcaps = {
 and guestcaps_block_type = Virtio_blk | Virtio_SCSI | IDE
 and guestcaps_net_type = Virtio_net | E1000 | RTL8139
 and guestcaps_video_type = QXL | Cirrus
+and guestcaps_machine = I440FX | Q35 | Virt
 
 val string_of_guestcaps : guestcaps -> string
 val string_of_requested_guestcaps : requested_guestcaps -> string
-- 
2.16.2

On Tue, Jun 19, 2018 at 11:37:46AM +0100, Richard W.M. Jones
wrote:
> QEMU for x86 supports two machine types, "pc" (emulating the
ancient
> Intel i440FX chipset originally used by the Pentium Pro), and
"q35"
> (https://wiki.qemu.org/Features/Q35).
> 
> Currently virt-v2v does not set any machine type, so libvirt or the
> target hypervisor will choose some default, probably i440fx.  Newer
> versions of libvirt and QEMU will probably switch over to defaulting
> to Q35 in the near future.
Actually, we seem to be arriving at the conclusion that libvirt is
explicitly *not* going to switch its default because of the impact
on apps using it. So from libvirt's POV we'll do our very best to
ensure 'pc' is always the default

That said, we cannot guarantee that all vendors will build QEMU
with 'pc' present. IOW there's a chance that someone might ship
QEMU with only 'q35' availab.e

So it is worth setting an explicit machine type so you're confident
of what you'll be running with.

I'd encourage apps to check the capabilities XML to see what
machine types are available.
> 
> None of this matters for reasonably new guests since they can boot
> with either chipset.  However there are some very old guests (notably
> Windows XP) which cannot handle Q35.
> 
> This commit changes virt-v2v so it always tries to specify the machine
> type explicitly (assuming the target supports that, and not all of
> them do).  We pivot around the year 2007 which happens to be the year
> that Q35 hardware was first available.  Any guest OS which was first
> shipped earlier than 2007 will be given i440fx.  Any guest OS from
> 2007 or later will be given Q35.
> 
> For non-x86 architectures we select the "virt" model which will
> probably only work for AArch64.  More work is needed for POWER.
> 
> A note about secure boot: Previous to this patch, if a guest used UEFI
> and we detected that UEFI needed secure boot support, then we forced
> the machine type to Q35 (which was wrong for non-x86 architectures).
> In this patch I removed the code to force Q35, since any guest which
> is using secure boot will be new enough that it'll be using Q35 anyway
> after this patch (on x86).
> ---
>  v2v/convert_linux.ml      | 20 ++++++++++++++++++++
>  v2v/convert_windows.ml    | 10 ++++++++++
>  v2v/create_libvirt_xml.ml | 21 ++++++++++++++-------
>  v2v/create_ovf.ml         |  2 ++
>  v2v/output_glance.ml      |  5 +++++
>  v2v/output_qemu.ml        | 14 ++++++++------
>  v2v/test-v2v-i-ova.xml    |  2 +-
>  v2v/types.ml              |  8 ++++++++
>  v2v/types.mli             |  6 ++++--
>  9 files changed, 72 insertions(+), 16 deletions(-)
> 
> diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
> index 02dc2fee2..4dbf2aa41 100644
> --- a/v2v/convert_linux.ml
> +++ b/v2v/convert_linux.ml
> @@ -122,6 +122,25 @@ let convert (g : G.guestfs) inspect source output
rcaps >
>      SELinux_relabel.relabel g;
>  
> +    (* Pivot on the year 2007.  Any Linux distro from earlier than
> +     * 2007 should use i440fx, anything 2007 or newer should use q35.
> +     *)
> +    let machine > +      match inspect.i_arch, inspect.i_distro,
inspect.i_major_version with
> +      | ("i386"|"x86_64"), "fedora", _ ->
Q35
> +      | ("i386"|"x86_64"),
("rhel"|"centos"|"scientificlinux"|
> +                           
"redhat-based"|"oraclelinux"), major ->
> +         if major <= 4 then I440FX else Q35
> +      | ("i386"|"x86_64"),
("sles"|"suse-based"|"opensuse"), major ->
> +         if major < 10 then I440FX else Q35
> +      | ("i386"|"x86_64"),
("debian"|"ubuntu"|"linuxmint"|
> +                            "kalilinux"), major ->
> +         if major < 4 then I440FX else Q35
> +
> +      (* reasonable default for all modern Linux kernels *)
> +      | ("i386"|"x86_64"), _, _ -> Q35
> +      | _ -> Virt in
> +
>      (* Return guest capabilities from the convert () function. *)
>      let guestcaps = {
>        gcaps_block_bus = block_type;
> @@ -130,6 +149,7 @@ let convert (g : G.guestfs) inspect source output rcaps
>        gcaps_virtio_rng = kernel.ki_supports_virtio_rng;
>        gcaps_virtio_balloon = kernel.ki_supports_virtio_balloon;
>        gcaps_isa_pvpanic = kernel.ki_supports_isa_pvpanic;
> +      gcaps_machine = machine;
>        gcaps_arch = Utils.kvm_arch inspect.i_arch;
>        gcaps_acpi = acpi;
>      } in
> diff --git a/v2v/convert_windows.ml b/v2v/convert_windows.ml
> index 163319545..97882c377 100644
> --- a/v2v/convert_windows.ml
> +++ b/v2v/convert_windows.ml
> @@ -212,6 +212,15 @@ let convert (g : G.guestfs) inspect source output
rcaps >          warning (f_"this guest has Anti-Virus (AV) software and
a new virtio block device driver was installed.  In some circumstances, AV may
prevent new drivers from working (resulting in a 7B boot error).  If this
happens, try disabling AV before doing the conversion.");
>      );
>  
> +    (* Pivot on the year 2007.  Any Windows version from earlier than
> +     * 2007 should use i440fx, anything 2007 or newer should use q35.
> +     * Luckily this coincides almost exactly with the release of NT 6.
> +     *)
> +    let machine > +      match inspect.i_arch, inspect.i_major_version
with
> +      | ("i386"|"x86_64"), major -> if major < 6
then I440FX else Q35
> +      | _ -> Virt in
> +
>      (* Return guest capabilities from the convert () function. *)
>      let guestcaps = {
>        gcaps_block_bus = block_driver;
> @@ -220,6 +229,7 @@ let convert (g : G.guestfs) inspect source output rcaps
>        gcaps_virtio_rng = virtio_rng_supported;
>        gcaps_virtio_balloon = virtio_ballon_supported;
>        gcaps_isa_pvpanic = isa_pvpanic_supported;
> +      gcaps_machine = machine;
>        gcaps_arch = Utils.kvm_arch inspect.i_arch;
>        gcaps_acpi = true;
>      } in
> diff --git a/v2v/create_libvirt_xml.ml b/v2v/create_libvirt_xml.ml
> index fbe90eeaa..4b36ffb8e 100644
> --- a/v2v/create_libvirt_xml.ml
> +++ b/v2v/create_libvirt_xml.ml
> @@ -86,10 +86,6 @@ let create_libvirt_xml ?pool source target_buses
guestcaps
>      | Some { Uefi.flags = flags }
>           when List.mem Uefi.UEFI_FLAG_SECURE_BOOT_REQUIRED flags ->
true
>      | _ -> false in
> -  (* Currently these are required by secure boot, but in theory they
> -   * might be independent properties.
> -   *)
> -  let machine_q35 = secure_boot_required in
>    let smm = secure_boot_required in
>  
>    (* We have the machine features of the guest when it was on the
> @@ -140,7 +136,18 @@ let create_libvirt_xml ?pool source target_buses
guestcaps
>  
>    (* The <os> section subelements. *)
>    let os_section > -    let machine = if machine_q35 then [
"machine", "q35" ] else [] in
> +    let os = ref [] in
> +
> +    let machine > +      match guestcaps.gcaps_machine with
> +      | I440FX -> "pc"
> +      | Q35 -> "q35"
> +      | Virt -> "virt" in
> +
> +    List.push_back os
> +                   (e "type" ["arch",
guestcaps.gcaps_arch;
> +                              "machine", machine]
> +                      [PCData "hvm"]);
>  
>      let loader >        match uefi_firmware with
> @@ -152,8 +159,8 @@ let create_libvirt_xml ?pool source target_buses
guestcaps
>               [ PCData code ];
>             e "nvram" ["template", vars_template] [] ]
in
>  
> -    (e "type" (["arch", guestcaps.gcaps_arch] @
machine) [PCData "hvm"])
> -    :: loader in
> +    List.push_back_list os loader;
> +    !os in
>  
>    List.push_back_list body [
>      e "os" [] os_section;
> diff --git a/v2v/create_ovf.ml b/v2v/create_ovf.ml
> index 9e0c772fd..81357b55e 100644
> --- a/v2v/create_ovf.ml
> +++ b/v2v/create_ovf.ml
> @@ -602,6 +602,8 @@ let rec create_ovf source targets guestcaps inspect
>                                       source.s_vcpu memsize_mb)]
>        ] in
>  
> +      (* XXX How to set machine type for Q35? *)
> +
>        List.push_back virtual_hardware_section_items (
>          e "Item" [] ([
>            e "rasd:Caption" [] [PCData (sprintf "%d virtual
cpu" source.s_vcpu)];
> diff --git a/v2v/output_glance.ml b/v2v/output_glance.ml
> index c334def42..96c31da59 100644
> --- a/v2v/output_glance.ml
> +++ b/v2v/output_glance.ml
> @@ -86,6 +86,11 @@ object
>          (match guestcaps.gcaps_video with
>           | QXL -> "qxl"
>           | Cirrus -> "cirrus");
> +        "hw_machine_type",
> +        (match guestcaps.gcaps_machine with
> +         | I440FX -> "pc"
> +         | Q35 -> "q35"
> +         | Virt -> "virt");
>          "architecture", guestcaps.gcaps_arch;
>          "hypervisor_type", "kvm";
>          "vm_mode", "hvm";
> diff --git a/v2v/output_qemu.ml b/v2v/output_qemu.ml
> index 952660de2..487363520 100644
> --- a/v2v/output_qemu.ml
> +++ b/v2v/output_qemu.ml
> @@ -61,12 +61,14 @@ object
>        | Some { Uefi.flags }
>             when List.mem Uefi.UEFI_FLAG_SECURE_BOOT_REQUIRED flags ->
true
>        | _ -> false in
> -    (* Currently these are required by secure boot, but in theory they
> -     * might be independent properties.
> -     *)
> -    let machine_q35 = secure_boot_required in
>      let smm = secure_boot_required in
>  
> +    let machine > +      match guestcaps.gcaps_machine with
> +      | I440FX -> "pc"
> +      | Q35 -> "q35"
> +      | Virt -> "virt" in
> +
>      (* Construct the command line.  Note that the [Qemuopts]
>       * module deals with shell and qemu comma quoting.
>       *)
> @@ -80,8 +82,8 @@ object
>  
>      flag "-no-user-config"; flag "-nodefaults";
>      arg "-name" source.s_name;
> -    arg_list "-machine" (if machine_q35 then ["q35"]
else [] @
> -                         if smm then ["smm=on"] else [] @
> +    arg_list "-machine" (machine ::
> +                         (if smm then ["smm=on"] else []) @
>                           ["accel=kvm:tcg"]);
>  
>      (match uefi_firmware with
> diff --git a/v2v/test-v2v-i-ova.xml b/v2v/test-v2v-i-ova.xml
> index 5a303b80a..15925ce6e 100644
> --- a/v2v/test-v2v-i-ova.xml
> +++ b/v2v/test-v2v-i-ova.xml
> @@ -10,7 +10,7 @@
>      <apic/>
>    </features>
>    <os>
> -    <type arch='x86_64'>hvm</type>
> +    <type arch='x86_64'
machine='q35'>hvm</type>
>    </os>
>    <on_poweroff>destroy</on_poweroff>
>    <on_reboot>restart</on_reboot>
> diff --git a/v2v/types.ml b/v2v/types.ml
> index da1192ec3..a569759c7 100644
> --- a/v2v/types.ml
> +++ b/v2v/types.ml
> @@ -398,6 +398,7 @@ type guestcaps = {
>    gcaps_virtio_rng : bool;
>    gcaps_virtio_balloon : bool;
>    gcaps_isa_pvpanic : bool;
> +  gcaps_machine : guestcaps_machine;
>    gcaps_arch : string;
>    gcaps_acpi : bool;
>  }
> @@ -409,6 +410,7 @@ and requested_guestcaps = {
>  and guestcaps_block_type = Virtio_blk | Virtio_SCSI | IDE
>  and guestcaps_net_type = Virtio_net | E1000 | RTL8139
>  and guestcaps_video_type = QXL | Cirrus
> +and guestcaps_machine = I440FX | Q35 | Virt
>  
>  let string_of_block_type = function
>    | Virtio_blk -> "virtio-blk"
> @@ -421,17 +423,23 @@ let string_of_net_type = function
>  let string_of_video = function
>    | QXL -> "qxl"
>    | Cirrus -> "cirrus"
> +let string_of_machine = function
> +  | I440FX -> "i440fx"
> +  | Q35 -> "q35"
> +  | Virt -> "virt"
>  
>  let string_of_guestcaps gcaps >    sprintf "\
>  gcaps_block_bus = %s
>  gcaps_net_bus = %s
>  gcaps_video = %s
> +gcaps_machine = %s
>  gcaps_arch = %s
>  gcaps_acpi = %b
>  " (string_of_block_type gcaps.gcaps_block_bus)
>    (string_of_net_type gcaps.gcaps_net_bus)
>    (string_of_video gcaps.gcaps_video)
> +  (string_of_machine gcaps.gcaps_machine)
>    gcaps.gcaps_arch
>    gcaps.gcaps_acpi
>  
> diff --git a/v2v/types.mli b/v2v/types.mli
> index 291707b74..3eb946886 100644
> --- a/v2v/types.mli
> +++ b/v2v/types.mli
> @@ -240,8 +240,9 @@ type guestcaps = {
>    gcaps_virtio_balloon : bool;  (** Guest supports virtio balloon. *)
>    gcaps_isa_pvpanic : bool;     (** Guest supports ISA pvpanic device. *)
>  
> -  gcaps_arch : string;      (** Architecture that KVM must emulate. *)
> -  gcaps_acpi : bool;        (** True if guest supports acpi. *)
> +  gcaps_machine : guestcaps_machine; (** Machine model. *)
> +  gcaps_arch : string;          (** Architecture that KVM must emulate. *)
> +  gcaps_acpi : bool;            (** True if guest supports acpi. *)
>  }
>  (** Guest capabilities after conversion.  eg. Was virtio found or
installed? *)
>  
> @@ -257,6 +258,7 @@ and requested_guestcaps = {
>  and guestcaps_block_type = Virtio_blk | Virtio_SCSI | IDE
>  and guestcaps_net_type = Virtio_net | E1000 | RTL8139
>  and guestcaps_video_type = QXL | Cirrus
> +and guestcaps_machine = I440FX | Q35 | Virt
>  
>  val string_of_guestcaps : guestcaps -> string
>  val string_of_requested_guestcaps : requested_guestcaps -> string
> -- 
> 2.16.2
> 
Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Tue, Jun 19, 2018 at 11:43:38AM +0100, Daniel P. Berrangé
wrote:
> I'd encourage apps to check the capabilities XML to see what
> machine types are available.
One issue is we don't always have access to the target hypervisor.

For example in the Glance case we have to write something which will
be picked up by Nova much later:
> > +        "hw_machine_type",
> > +        (match guestcaps.gcaps_machine with
> > +         | I440FX -> "pc"
> > +         | Q35 -> "q35"
> > +         | Virt -> "virt");
I read the Nova code and it seems very ad-hoc.  As far as I can tell
these strings are eventually passed down to libvirt.  However libvirt
capabilities doesn't advertise these machine types exactly, but
something more like "pc-q35-2.6".  Does libvirt map "q35" to
something
intelligent?

In any case it's not passed to libvirt before Nova has also done
various ad-hoc stuff based on substrings.  For example Nova will only
add PCIe root ports to a guest if the machine type has "q35" as a
substring.

I can see many ways this could all go horribly wrong.

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