Libguestfs - Dec 2017 - [PATCH v2 0/2] v2v: -o null: Use the qemu null device driver.

This changes the infrastructure to allow the target_file to be
a QEMU URI.

Rich.

This complicated bit of refactoring abstracts the target file so it is
allowed to be either a filename or a QEMU URI.
---
 v2v/create_libvirt_xml.ml |  9 +++--
 v2v/output_glance.ml      | 10 ++++--
 v2v/output_libvirt.ml     |  5 +--
 v2v/output_local.ml       |  5 +--
 v2v/output_null.ml        |  2 +-
 v2v/output_qemu.ml        | 17 ++++++---
 v2v/output_rhv.ml         |  7 +++-
 v2v/output_vdsm.ml        |  6 +++-
 v2v/types.ml              |  9 +++--
 v2v/types.mli             |  6 +++-
 v2v/v2v.ml                | 90 ++++++++++++++++++++++++++++++-----------------
 11 files changed, 116 insertions(+), 50 deletions(-)

diff --git a/v2v/create_libvirt_xml.ml b/v2v/create_libvirt_xml.ml
index ecf0d14c5..c6b47538e 100644
--- a/v2v/create_libvirt_xml.ml
+++ b/v2v/create_libvirt_xml.ml
@@ -178,6 +178,11 @@ let create_libvirt_xml ?pool source target_buses guestcaps
     | BusSlotEmpty -> Comment (sprintf "%s slot %d is empty"
bus_name i)
 
     | BusSlotTarget t ->
+       let target_file +         match t.target_file with
+         | TargetFile s -> s
+         | TargetURI _ -> assert false in
+
         e "disk" [
           "type", if pool = None then "file" else
"volume";
           "device", "disk"
@@ -190,12 +195,12 @@ let create_libvirt_xml ?pool source target_buses guestcaps
           (match pool with
           | None ->
             e "source" [
-              "file", absolute_path t.target_file;
+              "file", absolute_path target_file;
             ] []
           | Some pool ->
             e "source" [
               "pool", pool;
-              "volume", Filename.basename t.target_file;
+              "volume", Filename.basename target_file;
             ] []
           );
           e "target" [
diff --git a/v2v/output_glance.ml b/v2v/output_glance.ml
index d00011eb5..6f9defc3e 100644
--- a/v2v/output_glance.ml
+++ b/v2v/output_glance.ml
@@ -63,8 +63,8 @@ object
     (* Write targets to a temporary local file - see above for reason. *)
     List.map (
       fun t ->
-        let target_file = tmpdir // t.target_overlay.ov_sd in
-        { t with target_file = target_file }
+        let target_file = TargetFile (tmpdir // t.target_overlay.ov_sd) in
+        { t with target_file }
     ) targets
 
   method create_metadata source targets _ guestcaps inspect target_firmware @@
-157,6 +157,12 @@ object
               fun (k, v) -> [ "--property"; sprintf
"%s=%s" k v ]
             ) common_properties
           ) in
+
+        let target_file +          match target_file with
+          | TargetFile s -> s
+          | TargetURI _ -> assert false in
+
         let cmd = [ "glance"; "image-create";
"--name"; name;
                     "--disk-format=" ^ target_format;
                     "--container-format=bare"; "--file";
target_file;
diff --git a/v2v/output_libvirt.ml b/v2v/output_libvirt.ml
index 729f8b67a..ffa3e43c3 100644
--- a/v2v/output_libvirt.ml
+++ b/v2v/output_libvirt.ml
@@ -128,8 +128,9 @@ class output_libvirt oc output_pool = object
     List.map (
       fun t ->
         let target_file -          target_path // source.s_name ^ "-"
^ t.target_overlay.ov_sd in
-        { t with target_file = target_file }
+          TargetFile (target_path // source.s_name ^ "-" ^
+                      t.target_overlay.ov_sd) in
+        { t with target_file }
     ) targets
 
   method supported_firmware = [ TargetBIOS; TargetUEFI ]
diff --git a/v2v/output_local.ml b/v2v/output_local.ml
index 97ad8dddd..a6836be2f 100644
--- a/v2v/output_local.ml
+++ b/v2v/output_local.ml
@@ -34,8 +34,9 @@ class output_local dir = object
   method prepare_targets source targets      List.map (
       fun t ->
-        let target_file = dir // source.s_name ^ "-" ^
t.target_overlay.ov_sd in
-        { t with target_file = target_file }
+        let target_file +          TargetFile (dir // source.s_name ^
"-" ^ t.target_overlay.ov_sd) in
+        { t with target_file }
     ) targets
 
   method supported_firmware = [ TargetBIOS; TargetUEFI ]
diff --git a/v2v/output_null.ml b/v2v/output_null.ml
index d01f45654..65861a597 100644
--- a/v2v/output_null.ml
+++ b/v2v/output_null.ml
@@ -47,7 +47,7 @@ object
     List.map (
       fun t ->
         let target_file = tmpdir // t.target_overlay.ov_sd in
-        { t with target_file = target_file }
+        { t with target_file = TargetFile target_file }
     ) targets
 
   method create_metadata _ _ _ _ _ _ = ()
diff --git a/v2v/output_qemu.ml b/v2v/output_qemu.ml
index f61d698d6..f4c6b8b80 100644
--- a/v2v/output_qemu.ml
+++ b/v2v/output_qemu.ml
@@ -35,8 +35,9 @@ object
   method prepare_targets source targets      List.map (
       fun t ->
-        let target_file = dir // source.s_name ^ "-" ^
t.target_overlay.ov_sd in
-        { t with target_file = target_file }
+        let target_file +          TargetFile (dir // source.s_name ^
"-" ^ t.target_overlay.ov_sd) in
+        { t with target_file }
     ) targets
 
   method supported_firmware = [ TargetBIOS; TargetUEFI ]
@@ -122,7 +123,11 @@ object
     | BusSlotEmpty -> ()
 
     | BusSlotTarget t ->
-       arg_list "-drive" ["file=" ^ t.target_file;
"format=" ^ t.target_format;
+       let target_file +         match t.target_file with
+         | TargetFile s -> s
+         | TargetURI _ -> assert false in
+       arg_list "-drive" ["file=" ^ target_file;
"format=" ^ t.target_format;
                           "if=" ^ if_name; "index=" ^
string_of_int i;
                           "media=disk"]
 
@@ -141,7 +146,11 @@ object
     | BusSlotEmpty -> ()
 
     | BusSlotTarget t ->
-       arg_list "-drive" ["file=" ^ t.target_file;
"format=" ^ t.target_format;
+       let target_file +         match t.target_file with
+         | TargetFile s -> s
+         | TargetURI _ -> assert false in
+       arg_list "-drive" ["file=" ^ target_file;
"format=" ^ t.target_format;
                           "if=scsi"; "bus=0";
"unit=" ^ string_of_int i;
                           "media=disk"]
 
diff --git a/v2v/output_rhv.ml b/v2v/output_rhv.ml
index 2bcd988c1..58e084a5c 100644
--- a/v2v/output_rhv.ml
+++ b/v2v/output_rhv.ml
@@ -230,9 +230,10 @@ object
         fun ({ target_overlay = ov } as t, image_uuid, vol_uuid) ->
           let ov_sd = ov.ov_sd in
           let target_file = images_dir // image_uuid // vol_uuid in
+
           debug "RHV: will export %s to %s" ov_sd target_file;
 
-          { t with target_file = target_file }
+          { t with target_file = TargetFile target_file }
       ) (List.combine3 targets image_uuids vol_uuids) in
 
     (* Generate the .meta file associated with each volume. *)
@@ -241,6 +242,10 @@ object
         targets in
     List.iter (
       fun ({ target_file }, meta) ->
+        let target_file +          match target_file with
+          | TargetFile s -> s
+          | TargetURI _ -> assert false in
         let meta_filename = target_file ^ ".meta" in
         Changeuid.make_file changeuid_t meta_filename meta
     ) (List.combine targets metas);
diff --git a/v2v/output_vdsm.ml b/v2v/output_vdsm.ml
index d5911e80e..993261882 100644
--- a/v2v/output_vdsm.ml
+++ b/v2v/output_vdsm.ml
@@ -134,7 +134,7 @@ object
 
           debug "VDSM: will export %s to %s" ov_sd target_file;
 
-          { t with target_file = target_file }
+          { t with target_file = TargetFile target_file }
       ) (List.combine3 targets vdsm_params.image_uuids vdsm_params.vol_uuids)
in
 
     (* Generate the .meta files associated with each volume. *)
@@ -143,6 +143,10 @@ object
         vdsm_params.image_uuids targets in
     List.iter (
       fun ({ target_file }, meta) ->
+        let target_file +          match target_file with
+          | TargetFile s -> s
+          | TargetURI _ -> assert false in
         let meta_filename = target_file ^ ".meta" in
         with_open_out meta_filename (fun chan -> output_string chan meta)
     ) (List.combine targets metas);
diff --git a/v2v/types.ml b/v2v/types.ml
index 1ffb09440..be86f1b3b 100644
--- a/v2v/types.ml
+++ b/v2v/types.ml
@@ -293,12 +293,15 @@ ov_source = %s
     ov.ov_source.s_qemu_uri
 
 type target = {
-  target_file : string;
+  target_file : target_file;
   target_format : string;
   target_estimated_size : int64 option;
   target_actual_size : int64 option;
   target_overlay : overlay;
 }
+and target_file +  | TargetFile of string
+  | TargetURI of string
 
 let string_of_target t    sprintf "\
@@ -308,7 +311,9 @@ target_estimated_size = %s
 target_overlay = %s
 target_overlay.ov_source = %s
 "
-    t.target_file
+    (match t.target_file with
+     | TargetFile s -> "[file] " ^ s
+     | TargetURI s -> "[qemu] " ^ s)
     t.target_format
     (match t.target_estimated_size with
     | None -> "None" | Some i -> Int64.to_string i)
diff --git a/v2v/types.mli b/v2v/types.mli
index 9e2f8ceb4..95d890638 100644
--- a/v2v/types.mli
+++ b/v2v/types.mli
@@ -189,7 +189,7 @@ val string_of_overlay : overlay -> string
 (** {2 Target disks} *)
 
 type target = {
-  target_file : string;      (** Destination file. *)
+  target_file : target_file; (** Destination file or QEMU URI. *)
   target_format : string;    (** Destination format (eg. -of option). *)
 
   (* Note that the estimate is filled in by core v2v.ml code before
@@ -203,6 +203,10 @@ type target = {
 }
 (** Target disk. *)
 
+and target_file +  | TargetFile of string     (** Target is a file. *)
+  | TargetURI of string      (** Target is a QEMU URI. *)
+
 val string_of_target : target -> string
 
 (** {2 Guest firmware} *)
diff --git a/v2v/v2v.ml b/v2v/v2v.ml
index 9edc080ff..94146b596 100644
--- a/v2v/v2v.ml
+++ b/v2v/v2v.ml
@@ -362,7 +362,7 @@ and init_targets cmdline output source overlays           *
estimate_target_size will fill in the target_estimated_size field.
          * actual_target_size will fill in the target_actual_size field.
          *)
-        { target_file = ""; target_format = format;
+        { target_file = TargetFile ""; target_format = format;
           target_estimated_size = None;
           target_actual_size = None;
           target_overlay = ov }
@@ -665,15 +665,24 @@ and copy_targets cmdline targets input output    at_exit
(fun () ->
     if !delete_target_on_exit then (
       List.iter (
-        fun t -> try unlink t.target_file with _ -> ()
+        fun t ->
+          match t.target_file with
+          | TargetURI _ -> ()
+          | TargetFile s -> try unlink s with _ -> ()
       ) targets
     )
   );
   let nr_disks = List.length targets in
   List.mapi (
     fun i t ->
-      message (f_"Copying disk %d/%d to %s (%s)")
-        (i+1) nr_disks t.target_file t.target_format;
+      (match t.target_file with
+       | TargetFile s ->
+          message (f_"Copying disk %d/%d to %s (%s)")
+                  (i+1) nr_disks s t.target_format;
+       | TargetURI s ->
+          message (f_"Copying disk %d/%d to qemu URI %s (%s)")
+                  (i+1) nr_disks s t.target_format
+      );
       debug "%s" (string_of_target t);
 
       (* We noticed that qemu sometimes corrupts the qcow2 file on
@@ -693,33 +702,47 @@ and copy_targets cmdline targets input output         *)
       input#adjust_overlay_parameters t.target_overlay;
 
-      (* It turns out that libguestfs's disk creation code is
-       * considerably more flexible and easier to use than
-       * qemu-img, so create the disk explicitly using libguestfs
-       * then pass the 'qemu-img convert -n' option so qemu reuses
-       * the disk.
-       *
-       * Also we allow the output mode to actually create the disk
-       * image.  This lets the output mode set ownership and
-       * permissions correctly if required.
-       *)
-      (* What output preallocation mode should we use? *)
-      let preallocation -        match t.target_format, cmdline.output_alloc
with
-        | ("raw"|"qcow2"), Sparse -> Some
"sparse"
-        | ("raw"|"qcow2"), Preallocated -> Some
"full"
-        | _ -> None (* ignore -oa flag for other formats *) in
-      let compat -        match t.target_format with "qcow2" ->
Some "1.1" | _ -> None in
-      output#disk_create
-        t.target_file t.target_format t.target_overlay.ov_virtual_size
-        ?preallocation ?compat;
+      (match t.target_file with
+       | TargetFile filename ->
+          (* It turns out that libguestfs's disk creation code is
+           * considerably more flexible and easier to use than
+           * qemu-img, so create the disk explicitly using libguestfs
+           * then pass the 'qemu-img convert -n' option so qemu reuses
+           * the disk.
+           *
+           * Also we allow the output mode to actually create the disk
+           * image.  This lets the output mode set ownership and
+           * permissions correctly if required.
+           *)
+          (* What output preallocation mode should we use? *)
+          let preallocation +            match t.target_format,
cmdline.output_alloc with
+            | ("raw"|"qcow2"), Sparse -> Some
"sparse"
+            | ("raw"|"qcow2"), Preallocated -> Some
"full"
+            | _ -> None (* ignore -oa flag for other formats *) in
+          let compat +            match t.target_format with "qcow2"
-> Some "1.1" | _ -> None in
+          output#disk_create filename t.target_format
+                             t.target_overlay.ov_virtual_size
+                             ?preallocation ?compat
 
-      let cmd = [ Guestfs_config.qemu_img; "convert" ] @
+       | TargetURI _ ->
+          (* XXX For the moment we assume that qemu URI outputs
+           * need no special work.  We can change this in future.
+           *)
+          ()
+      );
+
+      let cmd +        let filename +          match t.target_file with
+          | TargetFile filename -> "file:" ^ filename
+          | TargetURI uri -> uri in
+        [ Guestfs_config.qemu_img; "convert" ] @
         (if not (quiet ()) then [ "-p" ] else []) @
         [ "-n"; "-f"; "qcow2"; "-O";
t.target_format ] @
         (if cmdline.compressed then [ "-c" ] else []) @
-        [ overlay_file; t.target_file ] in
+        [ overlay_file; filename ] in
       let start_time = gettimeofday () in
       if run_command cmd <> 0 then
         error (f_"qemu-img command failed, see earlier errors");
@@ -772,11 +795,14 @@ and copy_targets cmdline targets input output  
 (* Update the target_actual_size field in the target structure. *)
 and actual_target_size target -  let size -    (* Ignore errors because we want
to avoid failures after copying. *)
-    try Some (du target.target_file)
-    with Failure _ | Invalid_argument _ -> None in
-  { target with target_actual_size = size }
+  match target.target_file with
+  | TargetFile filename ->
+     let size +       (* Ignore errors because we want to avoid failures after
copying. *)
+       try Some (du filename)
+       with Failure _ | Invalid_argument _ -> None in
+     { target with target_actual_size = size }
+  | TargetURI _ -> target
 
 (* Save overlays if --debug-overlays option was used. *)
 and preserve_overlays overlays src_name -- 
2.13.2

Instead of writing to a temporary file and deleting it, use the null
block driver (the "null-co://" URI) in qemu to throw it away.
---
 v2v/output_null.ml | 43 +++++++++++++++++++++++++++++--------------
 v2v/virt-v2v.pod   |  4 ----
 2 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/v2v/output_null.ml b/v2v/output_null.ml
index 65861a597..4a58d0621 100644
--- a/v2v/output_null.ml
+++ b/v2v/output_null.ml
@@ -26,16 +26,23 @@ open Common_gettext.Gettext
 open Types
 open Utils
 
+(* Notes:
+ *
+ * This only happens to work because we run qemu-img convert
+ * with the -n [no create output] option, since null-co doesn't
+ * support creation.  If -n is removed in the main program then
+ * the tests will break very obviously.
+ *
+ * The null-co device is not zero-sized.  It actually has a fixed
+ * size (defaults to 2^30 I believe).
+ *
+ * qemu-img convert checks the output size and will fail if it's
+ * too small, so we have to set the size.  We could set it to
+ * match the input size but it's easier to set it to some huge
+ * size instead.
+ *)
+
 class output_null -  (* It would be nice to be able to write to /dev/null.
-   * Unfortunately qemu-img convert cannot do that.  Instead create a
-   * temporary directory which is always deleted at exit.
-   *)
-  let tmpdir -    let base_dir = (open_guestfs ())#get_cachedir () in
-    let t = Mkdtemp.temp_dir ~base_dir "null." in
-    rmdir_on_exit t;
-    t in
 object
   inherit output
 
@@ -44,11 +51,19 @@ object
   method supported_firmware = [ TargetBIOS; TargetUEFI ]
 
   method prepare_targets source targets -    List.map (
-      fun t ->
-        let target_file = tmpdir // t.target_overlay.ov_sd in
-        { t with target_file = TargetFile target_file }
-    ) targets
+    let json_params = [
+      "file.driver", JSON.String "null-co";
+      "file.size", JSON.String "1E";
+    ] in
+    let target_file = TargetURI ("json:" ^ JSON.string_of_doc
json_params) in
+
+    (* While it's not intended that output drivers can set the
+     * target_format field (thus overriding the -of option), in
+     * this special case of -o null it is reasonable.
+     *)
+    let target_format = "raw" in
+
+    List.map (fun t -> { t with target_file; target_format }) targets
 
   method create_metadata _ _ _ _ _ _ = ()
 end
diff --git a/v2v/virt-v2v.pod b/v2v/virt-v2v.pod
index 1cf58663b..503830c9d 100644
--- a/v2v/virt-v2v.pod
+++ b/v2v/virt-v2v.pod
@@ -1915,10 +1915,6 @@ This temporarily places a full copy of the output disks
in C<$TMPDIR>.
 You must ensure there is sufficient space in the output directory for
 the converted guest.
 
-=item I<-o null>
-
-This temporarily places a full copy of the output disks in C<$TMPDIR>.
-
 =back
 
 See also L</Minimum free space check in the host> below.
-- 
2.13.2

On Friday, 8 December 2017 09:39:33 CET Richard W.M. Jones
wrote:
> This changes the infrastructure to allow the target_file to be
> a QEMU URI.
LGTM.

Thanks,
-- 
Pino Toscano