Libguestfs - Jul 2018 - [PATCH] v2v: Model machine type explicitly.

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.  The
latest advice from the QEMU and libvirt communities is not to rely on
the default machine type but to specify what we need explicitly, but
it may also be that the libvirt configuration file has been changed to
set the default machine type to Q35 (either by the distro or the end
user).

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).  For x86_64 guests this patch always selects i440fx (pc).
In future we hope to get the correct machine type for the guest from
libosinfo but this is not available yet.

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

For secure boot we still have to force the machine type to Q35.  In a
future version this forcing can be removed since any guest which is
using secure boot will be new enough that it'll be using Q35 anyway
(on x86).
---
 v2v/convert_linux.ml      |  7 +++++++
 v2v/convert_windows.ml    |  7 +++++++
 v2v/create_libvirt_xml.ml | 37 +++++++++++++++++++++++++------------
 v2v/create_ovf.ml         |  2 ++
 v2v/output_glance.ml      |  5 +++++
 v2v/output_qemu.ml        | 30 +++++++++++++++++++-----------
 v2v/test-v2v-i-ova.xml    |  2 +-
 v2v/types.ml              |  8 ++++++++
 v2v/types.mli             |  6 ++++--
 9 files changed, 78 insertions(+), 26 deletions(-)

diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
index 02dc2fee2..726915875 100644
--- a/v2v/convert_linux.ml
+++ b/v2v/convert_linux.ml
@@ -122,6 +122,12 @@ let convert (g : G.guestfs) inspect source output rcaps  
     SELinux_relabel.relabel g;
 
+    (* XXX Look up this information in libosinfo in future. *)
+    let machine +      match inspect.i_arch with
+      | "i386"|"x86_64" -> I440FX
+      | _ -> Virt in
+
     (* Return guest capabilities from the convert () function. *)
     let guestcaps = {
       gcaps_block_bus = block_type;
@@ -130,6 +136,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..1e058136e 100644
--- a/v2v/convert_windows.ml
+++ b/v2v/convert_windows.ml
@@ -212,6 +212,12 @@ 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.");
     );
 
+    (* XXX Look up this information in libosinfo in future. *)
+    let machine +      match inspect.i_arch with
+      | "i386"|"x86_64" -> I440FX
+      | _ -> Virt in
+
     (* Return guest capabilities from the convert () function. *)
     let guestcaps = {
       gcaps_block_bus = block_driver;
@@ -220,6 +226,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 cf2cad0a2..ac07f9e9e 100644
--- a/v2v/create_libvirt_xml.ml
+++ b/v2v/create_libvirt_xml.ml
@@ -86,15 +86,17 @@ let create_libvirt_xml ?pool source target_buses guestcaps
     match target_firmware with
     | TargetBIOS -> None
     | TargetUEFI -> Some (find_uefi_firmware guestcaps.gcaps_arch) in
-  let secure_boot_required -    match uefi_firmware with
-    | 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 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
+        *)
+       Q35, true
+    | machine, _ ->
+       machine, false in
   let smm = secure_boot_required in
 
   (* We have the machine features of the guest when it was on the
@@ -145,7 +147,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 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
@@ -157,8 +170,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 2cf610333..92b13d827 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 07eee4cb3..01dc80089 100644
--- a/v2v/output_qemu.ml
+++ b/v2v/output_qemu.ml
@@ -56,17 +56,25 @@ object
       match target_firmware with
       | TargetBIOS -> None
       | TargetUEFI -> Some (find_uefi_firmware guestcaps.gcaps_arch) in
-    let secure_boot_required -      match uefi_firmware with
-      | 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 machine, secure_boot_required +      match guestcaps.gcaps_machine,
uefi_firmware with
+      | _, Some { Uefi.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
+          *)
+         Q35, true
+      | machine, _ ->
+         machine, false in
     let smm = secure_boot_required in
 
+    let machine +      match 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.
      *)
@@ -87,8 +95,8 @@ object
         arg_list "-device" ["vmgenid"; sprintf
"guid=%s" genid; "id=vmgenid0"]
     );
 
-    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..b277193a8 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='pc'>hvm</type>
   </os>
   <on_poweroff>destroy</on_poweroff>
   <on_reboot>restart</on_reboot>
diff --git a/v2v/types.ml b/v2v/types.ml
index 3e3f0fc26..900275834 100644
--- a/v2v/types.ml
+++ b/v2v/types.ml
@@ -411,6 +411,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;
 }
@@ -422,6 +423,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"
@@ -434,17 +436,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 891caeafe..71788a53f 100644
--- a/v2v/types.mli
+++ b/v2v/types.mli
@@ -258,8 +258,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? *)
 
@@ -275,6 +276,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.18.0