Libguestfs - Feb 2017 - [PATCH 1/4] v2v: Pass output object into the conversion module.

The bug is:

https://bugzilla.redhat.com/show_bug.cgi?id=1161019

This makes a few other minor refactorings to the code.

Rich.

Previously the Convert_linux conversion module depended on one feature
of the output module (#keep_serial_console).  This was extracted in an
ad-hoc way from the output module and passed as an extra parameter to
the conversion module.

Instead of doing it this way, just pass the output module into the
conversion module, so it can call output#keep_serial_console itself.

This is just a simplification of the existing code, but otherwise adds
no new features.
---
 v2v/convert_linux.ml   | 4 ++--
 v2v/convert_windows.ml | 2 +-
 v2v/modules_list.ml    | 5 +++--
 v2v/modules_list.mli   | 7 ++++---
 v2v/v2v.ml             | 7 +++----
 5 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
index 7c42791..7d65516 100644
--- a/v2v/convert_linux.ml
+++ b/v2v/convert_linux.ml
@@ -38,7 +38,7 @@ open Linux_kernels
 module G = Guestfs
 
 (* The conversion function. *)
-let rec convert ~keep_serial_console (g : G.guestfs) inspect source rcaps +let
rec convert (g : G.guestfs) inspect source output rcaps   
(*----------------------------------------------------------------------*)
   (* Inspect the guest first.  We already did some basic inspection in
    * the common v2v.ml code, but that has to deal with generic guests
@@ -1005,7 +1005,7 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps
   let kernel, virtio = configure_kernel () in
 
-  if keep_serial_console then (
+  if output#keep_serial_console then (
     configure_console ();
     bootloader#configure_console ();
   ) else (
diff --git a/v2v/convert_windows.ml b/v2v/convert_windows.ml
index e259c22..36f47c8 100644
--- a/v2v/convert_windows.ml
+++ b/v2v/convert_windows.ml
@@ -37,7 +37,7 @@ module G = Guestfs
  * time the Windows VM is booted on KVM.
  *)
 
-let convert ~keep_serial_console (g : G.guestfs) inspect source rcaps +let
convert (g : G.guestfs) inspect source output rcaps    (* Get the data
directory. *)
   let virt_tools_data_dir      try Sys.getenv "VIRT_TOOLS_DATA_DIR"
diff --git a/v2v/modules_list.ml b/v2v/modules_list.ml
index 13dfee2..3b3cf2e 100644
--- a/v2v/modules_list.ml
+++ b/v2v/modules_list.ml
@@ -27,9 +27,10 @@ and register_output_module name = push_front name
output_modules
 let input_modules () = List.sort compare !input_modules
 and output_modules () = List.sort compare !output_modules
 
+type inspection_fn = Types.inspect -> bool
+
 type conversion_fn -  keep_serial_console:bool ->
-  Guestfs.guestfs -> Types.inspect -> Types.source ->
+  Guestfs.guestfs -> Types.inspect -> Types.source -> Types.output
->
   Types.requested_guestcaps -> Types.guestcaps
 
 let convert_modules = ref []
diff --git a/v2v/modules_list.mli b/v2v/modules_list.mli
index 5db9227..100c54e 100644
--- a/v2v/modules_list.mli
+++ b/v2v/modules_list.mli
@@ -30,12 +30,13 @@ val input_modules : unit -> string list
 val output_modules : unit -> string list
 (** Return the list of output modules. *)
 
+type inspection_fn = Types.inspect -> bool
+
 type conversion_fn -  keep_serial_console:bool ->
-  Guestfs.guestfs -> Types.inspect -> Types.source ->
+  Guestfs.guestfs -> Types.inspect -> Types.source -> Types.output
->
   Types.requested_guestcaps -> Types.guestcaps
 
-val register_convert_module : (Types.inspect -> bool) -> string ->
conversion_fn -> unit
+val register_convert_module : inspection_fn -> string -> conversion_fn
-> unit
 (** [register_convert_module inspect_fn name fn] registers a
     conversion function [fn] that can accept any guest that matches
     the [inspect_fn] function. *)
diff --git a/v2v/v2v.ml b/v2v/v2v.ml
index ee00d2e..b9fb759 100644
--- a/v2v/v2v.ml
+++ b/v2v/v2v.ml
@@ -103,7 +103,6 @@ let rec main ()  
   (* Conversion. *)
   let guestcaps -    let keep_serial_console = output#keep_serial_console in
     let rcaps        match conversion_mode with
       | Copying _ ->
@@ -111,7 +110,7 @@ let rec main ()        | In_place ->
          rcaps_from_source source in
 
-    do_convert g inspect source keep_serial_console rcaps in
+    do_convert g inspect source output rcaps in
 
   g#umount_all ();
 
@@ -549,7 +548,7 @@ and check_target_free_space mpstats source targets output   
output#check_target_free_space source targets
 
 (* Conversion. *)
-and do_convert g inspect source keep_serial_console rcaps +and do_convert g
inspect source output rcaps    (match inspect.i_product_name with
   | "unknown" ->
     message (f_"Converting the guest to run on KVM")
@@ -564,7 +563,7 @@ and do_convert g inspect source keep_serial_console rcaps   
inspect.i_type inspect.i_distro in
   debug "picked conversion module %s" conversion_name;
   debug "requested caps: %s" (string_of_requested_guestcaps rcaps);
-  let guestcaps = convert ~keep_serial_console g inspect source rcaps in
+  let guestcaps = convert g inspect source output rcaps in
   debug "%s" (string_of_guestcaps guestcaps);
 
   (* Did we manage to install virtio drivers? *)
-- 
2.9.3

This just simplifies the code and should make no functional
difference.

Updates commit c466d4cb928fcfea37908cc66c90ac8430d6d4c6.
---
 v2v/convert_windows.ml | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/v2v/convert_windows.ml b/v2v/convert_windows.ml
index 36f47c8..0289273 100644
--- a/v2v/convert_windows.ml
+++ b/v2v/convert_windows.ml
@@ -55,19 +55,6 @@ let convert (g : G.guestfs) inspect source output rcaps      
pnp_wait_exe msg;
         None in
 
-  (* Check if either RHEV-APT or VMDP exists.  This is optional. *)
-  let tools = [`RhevApt, "rhev-apt.exe"; `VmdpExe,
"vmdp.exe"] in
-  let installer -    try
-      let t, tool = List.find (
-        fun (_, tool) -> Sys.file_exists (virt_tools_data_dir // tool)
-      ) tools in
-      Some (t, virt_tools_data_dir // tool)
-    with Not_found -> (
-      warning (f_"Neither rhev-apt.exe nor vmdp.exe can be found.  Unable
to install one of them.");
-      None
-    ) in
-
   (*----------------------------------------------------------------------*)
   (* Inspect the Windows guest. *)
 
@@ -211,11 +198,18 @@ let convert (g : G.guestfs) inspect source output rcaps  
   let rec configure_firstboot ()      wait_pnp ();
-    (match installer with
-     | None -> ()
-     | Some (`RhevApt, tool_path) -> configure_rhev_apt tool_path
-     | Some (`VmdpExe, tool_path) -> configure_vmdp tool_path
+
+    let tool_path = virt_tools_data_dir // "rhev-apt.exe" in
+    if Sys.file_exists tool_path then
+      configure_rhev_apt tool_path
+    else (
+      let tool_path = virt_tools_data_dir // "vmdp.exe" in
+      if Sys.file_exists tool_path then
+        configure_vmdp tool_path
+      else
+        warning (f_"Neither rhev-apt.exe nor vmdp.exe can be found. 
Unable to install one of them.");
     );
+
     unconfigure_xenpv ();
     unconfigure_prltools ()
 
-- 
2.9.3

---
 test-data/fake-virt-tools/Makefile.am | 3 ++-
 test-data/fake-virt-tools/vmdp.exe    | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)
 create mode 100644 test-data/fake-virt-tools/vmdp.exe

diff --git a/test-data/fake-virt-tools/Makefile.am
b/test-data/fake-virt-tools/Makefile.am
index 0f96ace..f2cef75 100644
--- a/test-data/fake-virt-tools/Makefile.am
+++ b/test-data/fake-virt-tools/Makefile.am
@@ -26,4 +26,5 @@ include $(top_srcdir)/subdir-rules.mk
 
 EXTRA_DIST = \
 	rhev-apt.exe \
-	rhsrvany.exe
+	rhsrvany.exe \
+	vmdp.exe
diff --git a/test-data/fake-virt-tools/vmdp.exe
b/test-data/fake-virt-tools/vmdp.exe
new file mode 100644
index 0000000..9ca881a
--- /dev/null
+++ b/test-data/fake-virt-tools/vmdp.exe
@@ -0,0 +1 @@
+This is a fake vmdp.exe, used for testing only
-- 
2.9.3

Thanks: Tingting Zheng.
---
 v2v/convert_windows.ml | 2 +-
 v2v/output_rhv.ml      | 3 +++
 v2v/output_vdsm.ml     | 3 +++
 v2v/types.ml           | 1 +
 v2v/types.mli          | 2 ++
 5 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/v2v/convert_windows.ml b/v2v/convert_windows.ml
index 0289273..7d81c00 100644
--- a/v2v/convert_windows.ml
+++ b/v2v/convert_windows.ml
@@ -200,7 +200,7 @@ let convert (g : G.guestfs) inspect source output rcaps     
wait_pnp ();
 
     let tool_path = virt_tools_data_dir // "rhev-apt.exe" in
-    if Sys.file_exists tool_path then
+    if output#install_rhev_apt && Sys.file_exists tool_path then
       configure_rhev_apt tool_path
     else (
       let tool_path = virt_tools_data_dir // "vmdp.exe" in
diff --git a/v2v/output_rhv.ml b/v2v/output_rhv.ml
index e42429c..f407b4e 100644
--- a/v2v/output_rhv.ml
+++ b/v2v/output_rhv.ml
@@ -121,6 +121,9 @@ object
    *)
   method keep_serial_console = false
 
+  (* rhev-apt.exe will be installed (if available). *)
+  method install_rhev_apt = true
+
   (* Export Storage Domain mountpoint and UUID. *)
   val mutable esd_mp = ""
   val mutable esd_uuid = ""
diff --git a/v2v/output_vdsm.ml b/v2v/output_vdsm.ml
index 8b3148b..bbebff7 100644
--- a/v2v/output_vdsm.ml
+++ b/v2v/output_vdsm.ml
@@ -56,6 +56,9 @@ object
    *)
   method keep_serial_console = false
 
+  (* rhev-apt.exe will be installed (if available). *)
+  method install_rhev_apt = true
+
   (* Data Domain mountpoint. *)
   val mutable dd_mp = ""
   val mutable dd_uuid = ""
diff --git a/v2v/types.ml b/v2v/types.ml
index 9d94dca..b314b13 100644
--- a/v2v/types.ml
+++ b/v2v/types.ml
@@ -486,4 +486,5 @@ class virtual output = object
   method disk_create = (open_guestfs ())#disk_create
   method virtual create_metadata : source -> target list -> target_buses
-> guestcaps -> inspect -> target_firmware -> unit
   method keep_serial_console = true
+  method install_rhev_apt = false
 end
diff --git a/v2v/types.mli b/v2v/types.mli
index 48e47b0..e10802c 100644
--- a/v2v/types.mli
+++ b/v2v/types.mli
@@ -371,5 +371,7 @@ class virtual output : object
       same signature as Guestfs#disk_create. *)
   method keep_serial_console : bool
   (** Whether this output supports serial consoles (RHV does not). *)
+  method install_rhev_apt : bool
+  (** True iff [rhev-apt.exe] should be installed (only for RHV). *)
 end
 (** Encapsulates all [-o], etc output arguments as an object. *)
-- 
2.9.3

This is a simpler version of patch #1 which uses subtyping to enforce
at compile time that the conversion module can only call a whitelist
of methods on the output object.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html