Libguestfs - Apr 2017 - [PATCH 2/2] v2v: -o glance: factorize common properties

When converting a guest with UEFI firmware, set the also
hw_firmware_type=uefi property for all the disks of the guest, so Nova
can properly boot the guest.
---
 v2v/output_glance.ml | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/v2v/output_glance.ml b/v2v/output_glance.ml
index b712d68..cfb19b4 100644
--- a/v2v/output_glance.ml
+++ b/v2v/output_glance.ml
@@ -41,7 +41,7 @@ object
 
   method as_options = "-o glance"
 
-  method supported_firmware = [ TargetBIOS ]
+  method supported_firmware = [ TargetBIOS; TargetUEFI ]
 
   method prepare_targets source targets      (* This does nothing useful except
to check that the user has
@@ -66,9 +66,6 @@ object
     ) targets
 
   method create_metadata source targets _ guestcaps inspect target_firmware -  
(* See #supported_firmware above. *)
-    assert (target_firmware = TargetBIOS);
-
     (* The first disk, assumed to be the system disk, will be called
      * "guestname".  Subsequent disks, assumed to be data disks,
      * will be called "guestname-disk2" etc.  The manual strongly
@@ -143,6 +140,11 @@ object
         (* XXX Neither memory balloon nor pvpanic are supported by
          * Glance at this time.
          *)
+        (match target_firmware with
+         | TargetBIOS -> ()
+         | TargetUEFI ->
+            push_back properties ("hw_firmware_type",
"uefi")
+        );
 
         let properties            List.flatten (
-- 
2.9.3

Many of the properties for disks are the same for all of them, so
collect them only once, instead of doing that for every disk.

Should be just code motion, with no behaviour change.
---
 v2v/output_glance.ml | 143 ++++++++++++++++++++++++++-------------------------
 1 file changed, 73 insertions(+), 70 deletions(-)

diff --git a/v2v/output_glance.ml b/v2v/output_glance.ml
index cfb19b4..3feb2e4 100644
--- a/v2v/output_glance.ml
+++ b/v2v/output_glance.ml
@@ -66,6 +66,78 @@ object
     ) targets
 
   method create_metadata source targets _ guestcaps inspect target_firmware +  
(* Collect the common properties for all the disks. *)
+    let min_ram = source.s_memory /^ 1024L /^ 1024L in
+    let common_properties +      let properties = ref [
+        "hw_disk_bus",
+        (match guestcaps.gcaps_block_bus with
+         | Virtio_blk -> "virtio"
+         | Virtio_SCSI -> "scsi"
+         | IDE -> "ide");
+        "hw_vif_model",
+        (match guestcaps.gcaps_net_bus with
+         | Virtio_net -> "virtio"
+         | E1000 -> "e1000"
+         | RTL8139 -> "rtl8139");
+        "hw_video_model",
+        (match guestcaps.gcaps_video with
+         | QXL -> "qxl"
+         | Cirrus -> "cirrus");
+        "architecture", guestcaps.gcaps_arch;
+        "hypervisor_type", "kvm";
+        "vm_mode", "hvm";
+        "os_type", inspect.i_type;
+        "os_distro",
+        (match inspect.i_distro with
+        (* http://docs.openstack.org/cli-reference/glance-property-keys.html *)
+         | "archlinux" -> "arch"
+         | "sles" -> "sled"
+         | x -> x (* everything else is the same in libguestfs and
OpenStack*)
+        )
+      ] in
+      if source.s_cpu_sockets <> None || source.s_cpu_cores <> None
||
+         source.s_cpu_threads <> None then (
+        push_back properties ("hw_cpu_sockets",
+                              match source.s_cpu_sockets with
+                              | None -> "1"
+                              | Some v -> string_of_int v);
+        push_back properties ("hw_cpu_cores",
+                              match source.s_cpu_cores with
+                              | None -> "1"
+                              | Some v -> string_of_int v);
+        push_back properties ("hw_cpu_threads",
+                              match source.s_cpu_threads with
+                              | None -> "1"
+                              | Some v -> string_of_int v);
+      )
+      else (
+        push_back properties ("hw_cpu_sockets", "1");
+        push_back properties ("hw_cpu_cores", string_of_int
source.s_vcpu);
+      );
+      (match guestcaps.gcaps_block_bus with
+       | Virtio_SCSI ->
+          push_back properties ("hw_scsi_model",
"virtio-scsi")
+       | Virtio_blk | IDE -> ()
+      );
+      (match inspect.i_major_version, inspect.i_minor_version with
+       | 0, 0 -> ()
+       | x, 0 -> push_back properties ("os_version", string_of_int
x)
+       | x, y -> push_back properties ("os_version", sprintf
"%d.%d" x y)
+      );
+      if guestcaps.gcaps_virtio_rng then
+        push_back properties ("hw_rng_model", "virtio");
+      (* XXX Neither memory balloon nor pvpanic are supported by
+       * Glance at this time.
+       *)
+      (match target_firmware with
+       | TargetBIOS -> ()
+       | TargetUEFI ->
+          push_back properties ("hw_firmware_type", "uefi")
+      );
+
+      !properties in
+
     (* The first disk, assumed to be the system disk, will be called
      * "guestname".  Subsequent disks, assumed to be data disks,
      * will be called "guestname-disk2" etc.  The manual strongly
@@ -77,80 +149,11 @@ object
           if i == 0 then source.s_name
           else sprintf "%s-disk%d" source.s_name (i+1) in
 
-        (* Set the properties (ie. metadata). *)
-        let min_ram = source.s_memory /^ 1024L /^ 1024L in
-        let properties = ref [
-          "hw_disk_bus",
-          (match guestcaps.gcaps_block_bus with
-           | Virtio_blk -> "virtio"
-           | Virtio_SCSI -> "scsi"
-           | IDE -> "ide");
-          "hw_vif_model",
-          (match guestcaps.gcaps_net_bus with
-           | Virtio_net -> "virtio"
-           | E1000 -> "e1000"
-           | RTL8139 -> "rtl8139");
-          "hw_video_model",
-          (match guestcaps.gcaps_video with
-           | QXL -> "qxl"
-           | Cirrus -> "cirrus");
-          "architecture", guestcaps.gcaps_arch;
-          "hypervisor_type", "kvm";
-          "vm_mode", "hvm";
-          "os_type", inspect.i_type;
-          "os_distro",
-          (match inspect.i_distro with
-          (* http://docs.openstack.org/cli-reference/glance-property-keys.html
*)
-           | "archlinux" -> "arch"
-           | "sles" -> "sled"
-           | x -> x (* everything else is the same in libguestfs and
OpenStack*)
-          )
-        ] in
-        if source.s_cpu_sockets <> None || source.s_cpu_cores <>
None ||
-           source.s_cpu_threads <> None then (
-          push_back properties ("hw_cpu_sockets",
-                                match source.s_cpu_sockets with
-                                | None -> "1"
-                                | Some v -> string_of_int v);
-          push_back properties ("hw_cpu_cores",
-                                match source.s_cpu_cores with
-                                | None -> "1"
-                                | Some v -> string_of_int v);
-          push_back properties ("hw_cpu_threads",
-                                match source.s_cpu_threads with
-                                | None -> "1"
-                                | Some v -> string_of_int v);
-        )
-        else (
-          push_back properties ("hw_cpu_sockets", "1");
-          push_back properties ("hw_cpu_cores", string_of_int
source.s_vcpu);
-        );
-        (match guestcaps.gcaps_block_bus with
-         | Virtio_SCSI ->
-            push_back properties ("hw_scsi_model",
"virtio-scsi")
-         | Virtio_blk | IDE -> ()
-        );
-        (match inspect.i_major_version, inspect.i_minor_version with
-         | 0, 0 -> ()
-         | x, 0 -> push_back properties ("os_version",
string_of_int x)
-         | x, y -> push_back properties ("os_version", sprintf
"%d.%d" x y)
-        );
-        if guestcaps.gcaps_virtio_rng then
-          push_back properties ("hw_rng_model", "virtio");
-        (* XXX Neither memory balloon nor pvpanic are supported by
-         * Glance at this time.
-         *)
-        (match target_firmware with
-         | TargetBIOS -> ()
-         | TargetUEFI ->
-            push_back properties ("hw_firmware_type",
"uefi")
-        );
-
         let properties            List.flatten (
             List.map (
               fun (k, v) -> [ "--property"; sprintf
"%s=%s" k v ]
-            ) !properties
+            ) common_properties
           ) in
         let cmd = [ "glance"; "image-create";
"--name"; name;
                     "--disk-format=" ^ target_format;
-- 
2.9.3

On Wed, Apr 26, 2017 at 04:36:26PM +0200, Pino Toscano
wrote:
> Many of the properties for disks are the same for all of them, so
> collect them only once, instead of doing that for every disk.
> 
> Should be just code motion, with no behaviour change.
ACK series.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW