Libguestfs - May 2018 - [PATCH] v2v: -o null: support older qemu-img (RHBZ#1580309)

Commit 4699c7b6e126e07c95b67fb95df58aed87a680dd converted the null
output to use the null-co qemu driver with a JSON URL syntax --
especially the latter is only available in newer versions of qemu.

Even if this output mode is mostly for testing, check at runtime whether
the null-co + JSON way is possible, falling back to the creation of
thrown-away temporary files as before.
---
 v2v/output_null.ml | 60 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 49 insertions(+), 11 deletions(-)

diff --git a/v2v/output_null.ml b/v2v/output_null.ml
index b93d53dc5..5a5473c0d 100644
--- a/v2v/output_null.ml
+++ b/v2v/output_null.ml
@@ -42,7 +42,37 @@ open Utils
  * size instead.
  *)
 
+let qemu_supports_null_co_device () +  (* We actually attempt to convert a raw
file to the null-co device. *)
+  let tmp = Filename.temp_file "v2vqemunullcotst" ".img" in
+  Unix.truncate tmp 1024;
+
+  let json = [
+    "file.driver", JSON.String "null-co";
+    "file.size", JSON.String "1E";
+  ] in
+
+  let cmd +    sprintf "qemu-img convert -n -f raw -O raw %s json:%s
>/dev/null%s"
+            (quote tmp)
+            (quote (JSON.string_of_doc ~fmt:JSON.Compact json))
+            (if verbose () then "" else " 2>&1") in
+  debug "%s" cmd;
+  let r = 0 = Sys.command cmd in
+  Unix.unlink tmp;
+  debug "qemu-img supports the null-co device: %b" r;
+  r
+
 class output_null +  (* Create a temporary directory which is always deleted at
exit,
+   * so we can put the drives there in case qemu does not support
+   * the null-co device.
+   *)
+  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
 
@@ -51,19 +81,27 @@ object
   method supported_firmware = [ TargetBIOS; TargetUEFI ]
 
   method prepare_targets source 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
+    if qemu_supports_null_co_device () then (
+      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
+      (* 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
+      List.map (fun t -> { t with target_file; target_format }) targets
+    ) else (
+      List.map (
+        fun t ->
+          let target_file = tmpdir // t.target_overlay.ov_sd in
+          { t with target_file = TargetFile target_file }
+      ) targets
+    )
 
   method create_metadata _ _ _ _ _ _ = ()
 end
-- 
2.17.0

On Mon, May 21, 2018 at 05:28:18PM +0200, Pino Toscano
wrote:
> Commit 4699c7b6e126e07c95b67fb95df58aed87a680dd converted the null
> output to use the null-co qemu driver with a JSON URL syntax --
> especially the latter is only available in newer versions of qemu.
> 
> Even if this output mode is mostly for testing, check at runtime whether
> the null-co + JSON way is possible, falling back to the creation of
> thrown-away temporary files as before.
> ---
>  v2v/output_null.ml | 60 +++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 49 insertions(+), 11 deletions(-)
> 
> diff --git a/v2v/output_null.ml b/v2v/output_null.ml
> index b93d53dc5..5a5473c0d 100644
> --- a/v2v/output_null.ml
> +++ b/v2v/output_null.ml
> @@ -42,7 +42,37 @@ open Utils
>   * size instead.
>   *)
>  
> +let qemu_supports_null_co_device () > +  (* We actually attempt to
convert a raw file to the null-co device. *)
> +  let tmp = Filename.temp_file "v2vqemunullcotst"
".img" in
> +  Unix.truncate tmp 1024;
> +
> +  let json = [
> +    "file.driver", JSON.String "null-co";
> +    "file.size", JSON.String "1E";
> +  ] in
> +
> +  let cmd > +    sprintf "qemu-img convert -n -f raw -O raw %s
json:%s >/dev/null%s"
> +            (quote tmp)
> +            (quote (JSON.string_of_doc ~fmt:JSON.Compact json))
> +            (if verbose () then "" else "
2>&1") in
> +  debug "%s" cmd;
> +  let r = 0 = Sys.command cmd in
> +  Unix.unlink tmp;
> +  debug "qemu-img supports the null-co device: %b" r;
> +  r
I wonder if for consistency we should put this into v2v/utils.ml, and
also add a test to v2v/v2v_unit_tests.ml ...  (although the test for
Utils.qemu_img_supports_offset_and_size is kinda useless).

Rest of the patch looks good to me, so ACK.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top