Libguestfs - Mar 2023 - [PATCH v2v] convert: Allow preferred block driver to be specified on the command line

This is just an outline patch, only compile tested.

It doesn't make changes to virt-v2v-in-place, but those would be the
same as made in v2v/v2v.ml.

It reuses the existing Types.guestcaps_block_type which is a bit ugly
but fairly practical.

I've made the change to the documentation, but it needs a test.

Rich.

---
 docs/virt-v2v.pod           | 10 ++++++++++
 convert/convert.mli         |  1 +
 convert/convert_linux.mli   |  3 ++-
 convert/convert_windows.mli |  3 ++-
 convert/convert.ml          |  9 ++++++---
 convert/convert_linux.ml    |  2 +-
 convert/convert_windows.ml  | 12 +++++++++++-
 in-place/in_place.ml        |  3 ++-
 inspector/inspector.ml      |  3 ++-
 v2v/v2v.ml                  | 12 +++++++++++-
 10 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/docs/virt-v2v.pod b/docs/virt-v2v.pod
index b458607d14..e096418b2c 100644
--- a/docs/virt-v2v.pod
+++ b/docs/virt-v2v.pod
@@ -207,6 +207,16 @@ The options are silently ignored for other input methods.
 
 See I<--network> below.
 
+=item B<--block-driver virtio-blk>
+
+=item B<--block-driver virtio-scsi>
+
+When choosing a block driver for Windows guests, prefer C<virtio-blk> or
+C<virtio-scsi>.  The default is C<virtio-blk>.
+
+Note this has no effect for Linux guests at the moment.  That may be
+added in future.
+
 =item B<--colors>
 
 =item B<--colours>
diff --git a/convert/convert.mli b/convert/convert.mli
index 3bd39e2da8..c63bf6f0da 100644
--- a/convert/convert.mli
+++ b/convert/convert.mli
@@ -17,6 +17,7 @@
  *)
 
 type options = {
+  block_driver : Types.guestcaps_block_type; (** [--block-driver] option *)
   keep_serial_console : bool;
   ks : Tools_utils.key_store;      (** [--key] option *)
   network_map : Networks.t;        (** [-b] and [-n] options *)
diff --git a/convert/convert_linux.mli b/convert/convert_linux.mli
index 6eb272e9b4..dc6968fe51 100644
--- a/convert/convert_linux.mli
+++ b/convert/convert_linux.mli
@@ -23,5 +23,6 @@
     Mint and Kali are supported by this module. *)
 
 val convert : Guestfs.guestfs -> Types.source -> Types.inspect ->
-              Firmware.i_firmware -> bool -> Types.static_ip list ->
+              Firmware.i_firmware -> Types.guestcaps_block_type ->
+              bool -> Types.static_ip list ->
               Types.guestcaps
diff --git a/convert/convert_windows.mli b/convert/convert_windows.mli
index 42dac9f50c..33a14f6596 100644
--- a/convert/convert_windows.mli
+++ b/convert/convert_windows.mli
@@ -21,5 +21,6 @@
     This module converts a Windows guest to run on KVM. *)
 
 val convert : Guestfs.guestfs -> Types.source -> Types.inspect ->
-              Firmware.i_firmware -> bool -> Types.static_ip list ->
+              Firmware.i_firmware -> Types.guestcaps_block_type ->
+              bool -> Types.static_ip list ->
               Types.guestcaps
diff --git a/convert/convert.ml b/convert/convert.ml
index 0aa0e5cd3c..fa34d2ed7f 100644
--- a/convert/convert.ml
+++ b/convert/convert.ml
@@ -31,6 +31,7 @@ open Utils
 module G = Guestfs
 
 type options = {
+  block_driver : guestcaps_block_type;
   keep_serial_console : bool;
   ks : key_store;
   network_map : Networks.t;
@@ -88,7 +89,7 @@ let rec convert dir options source    (* Conversion. *)
   let guestcaps      do_convert g source inspect i_firmware
-      options.keep_serial_console options.static_ips in
+      options.block_driver options.keep_serial_console options.static_ips in
 
   g#umount_all ();
 
@@ -221,7 +222,8 @@ and do_fstrim g inspect    ) fses
 
 (* Conversion. *)
-and do_convert g source inspect i_firmware keep_serial_console interfaces +and
do_convert g source inspect i_firmware
+               block_driver keep_serial_console interfaces    (match
inspect.i_product_name with
   | "unknown" ->
     message (f_"Converting the guest to run on KVM")
@@ -246,7 +248,8 @@ and do_convert g source inspect i_firmware
keep_serial_console interfaces           inspect.i_type inspect.i_distro in
   debug "picked conversion module %s" conversion_name;
   let guestcaps -    convert g source inspect i_firmware keep_serial_console
interfaces in
+    convert g source inspect i_firmware
+            block_driver keep_serial_console interfaces in
   debug "%s" (string_of_guestcaps guestcaps);
 
   (* Did we manage to install virtio drivers? *)
diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml
index d5c0f24dbb..0f25af0eab 100644
--- a/convert/convert_linux.ml
+++ b/convert/convert_linux.ml
@@ -34,7 +34,7 @@ open Linux_kernels
 module G = Guestfs
 
 (* The conversion function. *)
-let convert (g : G.guestfs) source inspect i_firmware keep_serial_console _
+let convert (g : G.guestfs) source inspect i_firmware _ keep_serial_console _  
(*----------------------------------------------------------------------*)
   (* Inspect the guest first.  We already did some basic inspection in
    * the common v2v.ml code, but that has to deal with generic guests
diff --git a/convert/convert_windows.ml b/convert/convert_windows.ml
index 9d8d271d05..f36b486359 100644
--- a/convert/convert_windows.ml
+++ b/convert/convert_windows.ml
@@ -38,7 +38,7 @@ module G = Guestfs
  * time the Windows VM is booted on KVM.
  *)
 
-let convert (g : G.guestfs) _ inspect i_firmware _ static_ips +let convert (g :
G.guestfs) _ inspect i_firmware block_driver _ static_ips   
(*----------------------------------------------------------------------*)
   (* Inspect the Windows guest. *)
 
@@ -47,6 +47,16 @@ let convert (g : G.guestfs) _ inspect i_firmware _ static_ips
*)
   let virtio_win      Inject_virtio_win.from_environment g inspect.i_root
Config.datadir in
+  (match block_driver with
+   | Virtio_blk -> () (* the default, no need to do anything *)
+(*
+   | Virtio_scsi ->
+      let drivers = Inject_virtio_win.get_block_driver_priority virtio_win in
+      let drivers = "vioscsi" :: drivers in
+      Inject_virtio_win.set_block_driver_priority virtio_win drivers
+*)
+   | IDE -> assert false (* not possible - but maybe ...? *)
+  );
 
   (* If the Windows guest appears to be using group policy.
    *
diff --git a/in-place/in_place.ml b/in-place/in_place.ml
index 68ef99656d..16dacfbd27 100644
--- a/in-place/in_place.ml
+++ b/in-place/in_place.ml
@@ -294,7 +294,8 @@ read the man page virt-v2v-in-place(1).
 
   (* Get the conversion options. *)
   let conv_options = {
-    Convert.keep_serial_console = true;
+    Convert.block_driver = Virtio_blk; (* XXX *)
+    keep_serial_console = true;
     ks = opthandle.ks;
     network_map;
     root_choice;
diff --git a/inspector/inspector.ml b/inspector/inspector.ml
index a6428946ae..02d1a0e71e 100644
--- a/inspector/inspector.ml
+++ b/inspector/inspector.ml
@@ -324,7 +324,8 @@ read the man page virt-v2v-inspector(1).
 
   (* Get the conversion options. *)
   let conv_options = {
-    Convert.keep_serial_console = true;
+    Convert.block_driver = Virtio_blk;
+    keep_serial_console = true;
     ks = opthandle.ks;
     network_map;
     root_choice;
diff --git a/v2v/v2v.ml b/v2v/v2v.ml
index 13fe477a70..7148233b91 100644
--- a/v2v/v2v.ml
+++ b/v2v/v2v.ml
@@ -48,6 +48,7 @@ let rec main ()  
   let bandwidth = ref None in
   let bandwidth_file = ref None in
+  let block_driver = ref None in
   let input_conn = ref None in
   let input_format = ref None in
   let input_password = ref None in
@@ -230,6 +231,8 @@ let rec main ()                                     
s_"Set bandwidth dynamically from file";
     [ S 'b'; L"bridge" ], Getopt.String ("in:out",
add_bridge),
       s_"Map bridge ?in? to ?out?";
+    [ L"block-driver" ], Getopt.String ("driver",
set_string_option_once "--block-driver" block_driver),
+                                    s_"Prefer 'virtio-blk' or
'virtio-scsi'";
     [ L"compressed" ], Getopt.Unit (fun () ->
set_output_option_compat "compressed" ""),
       s_"Compress output file (-of qcow2 only)";
     [ S 'i' ],       Getopt.String
("disk|libvirt|libvirtxml|ova|vmx", set_input_mode),
@@ -351,6 +354,12 @@ read the man page virt-v2v(1).
 
   (* Dereference the arguments. *)
   let args = List.rev !args in
+  let block_driver +    match !block_driver with
+    | None | Some "virtio-blk" -> Virtio_blk
+    (* | Some "virtio-scsi" -> Virtio_scsi *)
+    | Some driver ->
+       error (f_"unknown block driver ?--block-driver %s?") driver in
   let input_conn = !input_conn in
   let input_mode = !input_mode in
   let input_transport @@ -524,7 +533,8 @@ read the man page virt-v2v(1).
 
   (* Get the conversion options. *)
   let conv_options = {
-    Convert.keep_serial_console = not remove_serial_console;
+    Convert.block_driver;
+    keep_serial_console = not remove_serial_console;
     ks = opthandle.ks;
     network_map;
     root_choice;
-- 
2.39.2

On 3/6/23 18:52, Richard W.M. Jones wrote:
> This is just an outline patch, only compile tested.
> 
> It doesn't make changes to virt-v2v-in-place, but those would be the
> same as made in v2v/v2v.ml.
> 
> It reuses the existing Types.guestcaps_block_type which is a bit ugly
> but fairly practical.
> 
> I've made the change to the documentation, but it needs a test.
> 
> Rich.
> 
Thanks for the provided outlines, I adapted them in my v2:

https://listman.redhat.com/archives/libguestfs/2023-March/030987.html
https://listman.redhat.com/archives/libguestfs/2023-March/030989.html

I have tested the in-place mode, and the switch seems to be working as
expected.

The patch also adds the convert option to the virt-v2v-inspector tool as
it's also using the conversion mechanism.  I figured that this cmdline
switch isn't much needed there, so the option is set just for avoiding
compilation issues.