On 11/08/21 10:12, Richard W.M. Jones wrote:
 This was part of the old in-place support.  When we add new in-place
 support we'll do something else, but currently this is dead code so
 remove it completely.
 
 Note this removes the code for installing the virtio-scsi driver (only
 ever using virtio-blk).  This was also dead code in the current
 implementation of virt-v2v, but worth remembering in case we want to
 resurrect this feature in a future version.
 ---
  convert/convert.ml          | 10 ++---
  convert/convert_linux.ml    | 24 +++-------
  convert/convert_linux.mli   |  2 +-
  convert/convert_windows.ml  |  4 +-
  convert/convert_windows.mli |  2 +-
  convert/windows_virtio.ml   | 87 ++++++-------------------------------
  convert/windows_virtio.mli  |  9 +---
  lib/types.ml                | 20 ---------
  lib/types.mli               | 10 -----
  9 files changed, 30 insertions(+), 138 deletions(-) 
This is exactly the kind of enormous patch that I don't trust myself to
implement in a project I'm new to. It's difficult to find the boundaries
(= where we stop cutting), and also difficult to find everything that
becomes unused / dead after the removals. (For example, I had to check
that "string_of_video" remained in use, via a different call path.)
I also find reviewing this pretty hard. The patch modifies many
(arguably) disparate aspects that I can't *all* keep easily in mind
during review. I think I might have attempted a more bottom-up, gradual
elimination approach, but that's exactly because I must watch my step
very closely. I do think this patch is fine.
Acked-by: Laszlo Ersek <lersek(a)redhat.com>
Thanks!
Laszlo
 
 diff --git a/convert/convert.ml b/convert/convert.ml
 index 07c7aee90..109e55284 100644
 --- a/convert/convert.ml
 +++ b/convert/convert.ml
 @@ -230,10 +230,7 @@ helper-v2v-convert V2VDIR
  
    (* Conversion. *)
    let guestcaps =
 -    let rcaps = (*XXX*)
 -      { rcaps_block_bus = None; rcaps_net_bus = None; rcaps_video = None } in
 -
 -    do_convert g source inspect keep_serial_console rcaps static_ips in
 +    do_convert g source inspect keep_serial_console static_ips in
  
    g#umount_all ();
  
 @@ -364,7 +361,7 @@ and do_fstrim g inspect =
    ) fses
  
  (* Conversion. *)
 -and do_convert g source inspect keep_serial_console rcaps interfaces =
 +and do_convert g source inspect keep_serial_console interfaces =
    (match inspect.i_product_name with
    | "unknown" ->
      message (f_"Converting the guest to run on KVM")
 @@ -388,9 +385,8 @@ and do_convert g source inspect keep_serial_console rcaps interfaces
=
         error (f_"virt-v2v is unable to convert this guest type (%s/%s)")
           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 g source inspect keep_serial_console rcaps interfaces in
 +    convert g source inspect 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 057cf9dff..b0b5d916d 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 keep_serial_console rcaps _ =
 +let convert (g : G.guestfs) source inspect 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
 @@ -111,22 +111,12 @@ let convert (g : G.guestfs) source inspect keep_serial_console
rcaps _ =
  
      let acpi = supports_acpi () in
  
 -    let video =
 -      match rcaps.rcaps_video with
 -      | None -> QXL
 -      | Some video -> video in
 -
      let block_type =
 -      match rcaps.rcaps_block_bus with
 -      | None -> if kernel.ki_supports_virtio_blk then Virtio_blk else IDE
 -      | Some block_type -> block_type in
 -
 +      if kernel.ki_supports_virtio_blk then Virtio_blk else IDE in
      let net_type =
 -      match rcaps.rcaps_net_bus with
 -      | None -> if kernel.ki_supports_virtio_net then Virtio_net else E1000
 -      | Some net_type -> net_type in
 +      if kernel.ki_supports_virtio_net then Virtio_net else E1000 in
  
 -    configure_display_driver video;
 +    configure_display_driver ();
      remap_block_devices block_type;
      configure_kernel_modules block_type net_type;
      rebuild_initrd kernel;
 @@ -158,7 +148,7 @@ let convert (g : G.guestfs) source inspect keep_serial_console rcaps
_ =
      let guestcaps = {
        gcaps_block_bus = block_type;
        gcaps_net_bus = net_type;
 -      gcaps_video = video;
 +      gcaps_video = QXL;
        gcaps_virtio_rng = kernel.ki_supports_virtio_rng;
        gcaps_virtio_balloon = kernel.ki_supports_virtio_balloon;
        gcaps_isa_pvpanic = kernel.ki_supports_isa_pvpanic;
 @@ -828,8 +818,8 @@ let convert (g : G.guestfs) source inspect keep_serial_console rcaps
_ =
      else
        true
  
 -  and configure_display_driver video =
 -    let video_driver = match video with QXL -> "qxl" | Cirrus ->
"cirrus" in
 +  and configure_display_driver () =
 +    let video_driver = "qxl" in
  
      let updated = ref false in
  
 diff --git a/convert/convert_linux.mli b/convert/convert_linux.mli
 index 8adc172a0..688a7acea 100644
 --- a/convert/convert_linux.mli
 +++ b/convert/convert_linux.mli
 @@ -23,5 +23,5 @@
      Mint and Kali are supported by this module. *)
  
  val convert : Guestfs.guestfs -> Types.source -> Types.inspect ->
 -              bool -> Types.requested_guestcaps -> Types.static_ip list ->
 +              bool -> Types.static_ip list ->
                Types.guestcaps
 diff --git a/convert/convert_windows.ml b/convert/convert_windows.ml
 index f2c1132bf..31e16723b 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 _ rcaps static_ips =
 +let convert (g : G.guestfs) _ inspect _ static_ips =
    (*----------------------------------------------------------------------*)
    (* Inspect the Windows guest. *)
  
 @@ -469,7 +469,7 @@ if errorlevel 3010 exit /b 0
      disable_xenpv_win_drivers reg;
      disable_prl_drivers reg;
      disable_autoreboot reg;
 -    Windows_virtio.install_drivers reg inspect rcaps
 +    Windows_virtio.install_drivers reg inspect
  
    and disable_xenpv_win_drivers reg =
      (* Disable xenpv-win service (RHBZ#809273). *)
 diff --git a/convert/convert_windows.mli b/convert/convert_windows.mli
 index d2978d6a4..d3ea3d6ae 100644
 --- a/convert/convert_windows.mli
 +++ b/convert/convert_windows.mli
 @@ -21,5 +21,5 @@
      This module converts a Windows guest to run on KVM. *)
  
  val convert : Guestfs.guestfs -> Types.source -> Types.inspect ->
 -              bool -> Types.requested_guestcaps -> Types.static_ip list ->
 +              bool -> Types.static_ip list ->
                Types.guestcaps
 diff --git a/convert/windows_virtio.ml b/convert/windows_virtio.ml
 index f843dae2d..08db4ad69 100644
 --- a/convert/windows_virtio.ml
 +++ b/convert/windows_virtio.ml
 @@ -44,31 +44,16 @@ let viostor_modern_pciid =
"VEN_1AF4&DEV_1042&SUBSYS_11001AF4&REV_01"
  let vioscsi_legacy_pciid =
"VEN_1AF4&DEV_1004&SUBSYS_00081AF4&REV_00"
  let vioscsi_modern_pciid =
"VEN_1AF4&DEV_1048&SUBSYS_11001AF4&REV_01"
  
 -let rec install_drivers ((g, _) as reg) inspect rcaps =
 +let rec install_drivers ((g, _) as reg) inspect =
    (* Copy the virtio drivers to the guest. *)
    let driverdir = sprintf "%s/Drivers/VirtIO" inspect.i_windows_systemroot in
    g#mkdir_p driverdir;
  
    if not (copy_drivers g inspect driverdir) then (
 -    match rcaps with
 -    | { rcaps_block_bus = Some Virtio_blk | Some Virtio_SCSI }
 -    | { rcaps_net_bus = Some Virtio_net }
 -    | { rcaps_video = Some QXL } ->
 -      error (f_"there are no virtio drivers available for this version of Windows
(%d.%d %s %s).  virt-v2v looks for drivers in %s")
 -            inspect.i_major_version inspect.i_minor_version inspect.i_arch
 -            inspect.i_product_variant virtio_win
 -
 -    | { rcaps_block_bus = (Some IDE | None);
 -        rcaps_net_bus = ((Some E1000 | Some RTL8139 | None) as net_type);
 -        rcaps_video = (Some Cirrus | None) } ->
        warning (f_"there are no virtio drivers available for this version of Windows
(%d.%d %s %s).  virt-v2v looks for drivers in %s\n\nThe guest will be configured to use
slower emulated devices.")
                inspect.i_major_version inspect.i_minor_version inspect.i_arch
                inspect.i_product_variant virtio_win;
 -      let net_type =
 -        match net_type with
 -        | Some model -> model
 -        | None -> RTL8139 in
 -      (IDE, net_type, Cirrus, false, false, false, false)
 +      (IDE, RTL8139, Cirrus, false, false, false, false)
    )
    else (
      (* Can we install the block driver? *)
 @@ -83,25 +68,14 @@ let rec install_drivers ((g, _) as reg) inspect rcaps =
            ) filenames
          )
        ) with Not_found -> None in
 -      let has_vioscsi = g#exists (driverdir // "vioscsi.inf") in
 -      match rcaps.rcaps_block_bus, viostor_driver, has_vioscsi with
 -      | Some Virtio_blk, None, _ ->
 -        error (f_"there is no virtio block device driver for this version of
Windows (%d.%d %s).  virt-v2v looks for this driver in %s\n\nThe guest will be configured
to use a slower emulated device.")
 -              inspect.i_major_version inspect.i_minor_version
 -              inspect.i_arch virtio_win
 -
 -      | Some Virtio_SCSI, _, false ->
 -        error (f_"there is no vioscsi (virtio SCSI) driver for this version of
Windows (%d.%d %s).  virt-v2v looks for this driver in %s\n\nThe guest will be configured
to use a slower emulated device.")
 -              inspect.i_major_version inspect.i_minor_version
 -              inspect.i_arch virtio_win
 -
 -      | None, None, _ ->
 +      match viostor_driver with
 +      | None ->
          warning (f_"there is no virtio block device driver for this version of
Windows (%d.%d %s).  virt-v2v looks for this driver in %s\n\nThe guest will be configured
to use a slower emulated device.")
                  inspect.i_major_version inspect.i_minor_version
                  inspect.i_arch virtio_win;
          IDE
  
 -      | (Some Virtio_blk | None), Some driver_name, _ ->
 +      | Some driver_name ->
          (* Block driver needs tweaks to allow booting; the rest is set up by PnP
           * manager *)
          let source = driverdir // (driver_name ^ ".sys") in
 @@ -111,22 +85,7 @@ let rec install_drivers ((g, _) as reg) inspect rcaps =
          g#cp source target;
          add_guestor_to_registry reg inspect driver_name viostor_legacy_pciid;
          add_guestor_to_registry reg inspect driver_name viostor_modern_pciid;
 -        Virtio_blk
 -
 -      | Some Virtio_SCSI, _, true ->
 -        (* Block driver needs tweaks to allow booting; the rest is set up by PnP
 -         * manager *)
 -        let source = driverdir // "vioscsi.sys" in
 -        let target = sprintf "%s/system32/drivers/vioscsi.sys"
 -                             inspect.i_windows_systemroot in
 -        let target = g#case_sensitive_path target in
 -        g#cp source target;
 -        add_guestor_to_registry reg inspect "vioscsi" vioscsi_legacy_pciid;
 -        add_guestor_to_registry reg inspect "vioscsi" vioscsi_modern_pciid;
 -        Virtio_SCSI
 -
 -      | Some IDE, _, _ ->
 -        IDE in
 +        Virtio_blk in
  
      (* Can we install the virtio-net driver? *)
      let net : guestcaps_net_type =
 @@ -135,46 +94,28 @@ let rec install_drivers ((g, _) as reg) inspect rcaps =
          List.exists (
            fun driver_file -> g#exists (driverdir // driver_file)
          ) filenames in
 -      match rcaps.rcaps_net_bus, has_netkvm with
 -      | Some Virtio_net, false ->
 -        error (f_"there is no virtio network driver for this version of Windows
(%d.%d %s).  virt-v2v looks for this driver in %s")
 -              inspect.i_major_version inspect.i_minor_version
 -              inspect.i_arch virtio_win
 -
 -      | None, false ->
 +      if not has_netkvm then (
          warning (f_"there is no virtio network driver for this version of Windows
(%d.%d %s).  virt-v2v looks for this driver in %s\n\nThe guest will be configured to use a
slower emulated device.")
                  inspect.i_major_version inspect.i_minor_version
                  inspect.i_arch virtio_win;
          RTL8139
 -
 -      | (Some Virtio_net | None), true ->
 -        Virtio_net
 -
 -      | Some net_type, _ ->
 -        net_type in
 +      )
 +      else
 +        Virtio_net in
  
      (* Can we install the QXL driver? *)
      let video : guestcaps_video_type =
        let has_qxl =
          g#exists (driverdir // "qxl.inf") ||
          g#exists (driverdir // "qxldod.inf") in
 -      match rcaps.rcaps_video, has_qxl with
 -      | Some QXL, false ->
 -        error (f_"there is no QXL driver for this version of Windows (%d.%d %s). 
virt-v2v looks for this driver in %s")
 -              inspect.i_major_version inspect.i_minor_version
 -              inspect.i_arch virtio_win
 -
 -      | None, false ->
 +      if not has_qxl then (
          warning (f_"there is no QXL driver for this version of Windows (%d.%d %s). 
virt-v2v looks for this driver in %s\n\nThe guest will be configured to use a basic VGA
display driver.")
                  inspect.i_major_version inspect.i_minor_version
                  inspect.i_arch virtio_win;
          Cirrus
 -
 -      | (Some QXL | None), true ->
 -        QXL
 -
 -      | Some Cirrus, _ ->
 -        Cirrus in
 +      )
 +      else
 +        QXL in
  
      (* Did we install the miscellaneous drivers? *)
      let virtio_rng_supported = g#exists (driverdir // "viorng.inf") in
 diff --git a/convert/windows_virtio.mli b/convert/windows_virtio.mli
 index 642317b15..4e24625a4 100644
 --- a/convert/windows_virtio.mli
 +++ b/convert/windows_virtio.mli
 @@ -19,9 +19,9 @@
  (** Functions for installing Windows virtio drivers. *)
  
  val install_drivers
 -    : Registry.t -> Types.inspect -> Types.requested_guestcaps ->
 +    : Registry.t -> Types.inspect ->
        Types.guestcaps_block_type * Types.guestcaps_net_type * Types.guestcaps_video_type
* bool * bool * bool * bool
 -(** [install_drivers reg inspect rcaps]
 +(** [install_drivers reg inspect]
      installs virtio drivers from the driver directory or driver
      ISO into the guest driver directory and updates the registry
      so that the [viostor.sys] driver gets loaded by Windows at boot.
 @@ -29,11 +29,6 @@ val install_drivers
      [reg] is the system hive which is open for writes when this
      function is called.
  
 -    [rcaps] is the set of guest "capabilities" requested by the caller.  This
 -    may include the type of the block driver, network driver, and video driver.
 -    install_drivers will adjust its choices based on that information, and
 -    abort if the requested driver wasn't found.
 -
      This returns the tuple [(block_driver, net_driver, video_driver,
      virtio_rng_supported, virtio_ballon_supported, isa_pvpanic_supported)]
      reflecting what devices are now required by the guest, either
 diff --git a/lib/types.ml b/lib/types.ml
 index 72d10d1ec..9be3e6fcd 100644
 --- a/lib/types.ml
 +++ b/lib/types.ml
 @@ -422,11 +422,6 @@ type guestcaps = {
    gcaps_arch : string;
    gcaps_acpi : bool;
  }
 -and requested_guestcaps = {
 -  rcaps_block_bus : guestcaps_block_type option;
 -  rcaps_net_bus : guestcaps_net_type option;
 -  rcaps_video : guestcaps_video_type option;
 -}
  and guestcaps_block_type = Virtio_blk | Virtio_SCSI | IDE
  and guestcaps_net_type = Virtio_net | E1000 | RTL8139
  and guestcaps_video_type = QXL | Cirrus
 @@ -463,21 +458,6 @@ gcaps_acpi = %b
    gcaps.gcaps_arch
    gcaps.gcaps_acpi
  
 -let string_of_requested_guestcaps rcaps =
 -  sprintf "\
 -rcaps_block_bus = %s
 -rcaps_net_bus = %s
 -rcaps_video = %s
 -" (match rcaps.rcaps_block_bus with
 -   | None -> "unspecified"
 -   | Some block_type -> (string_of_block_type block_type))
 -  (match rcaps.rcaps_net_bus with
 -   | None -> "unspecified"
 -   | Some net_type -> (string_of_net_type net_type))
 -  (match rcaps.rcaps_video with
 -   | None -> "unspecified"
 -   | Some video -> (string_of_video video))
 -
  type target_buses = {
    target_virtio_blk_bus : target_bus_slot array;
    target_ide_bus : target_bus_slot array;
 diff --git a/lib/types.mli b/lib/types.mli
 index 0bae445d8..4d4049605 100644
 --- a/lib/types.mli
 +++ b/lib/types.mli
 @@ -282,22 +282,12 @@ type guestcaps = {
  }
  (** Guest capabilities after conversion.  eg. Was virtio found or installed? *)
  
 -and requested_guestcaps = {
 -  rcaps_block_bus : guestcaps_block_type option;
 -  rcaps_net_bus : guestcaps_net_type option;
 -  rcaps_video : guestcaps_video_type option;
 -}
 -(** For [--in-place] conversions, the requested guest capabilities, to
 -    allow the caller to affect conversion choices.  [None] = no
 -    preference, use the best available. *)
 -
  and guestcaps_block_type = Virtio_blk | Virtio_SCSI | IDE
  and guestcaps_net_type = Virtio_net | E1000 | RTL8139
  and guestcaps_video_type = QXL | Cirrus
  and guestcaps_machine = I440FX | Q35 | Virt
  
  val string_of_guestcaps : guestcaps -> string
 -val string_of_requested_guestcaps : requested_guestcaps -> string
  
  (** {2 Guest buses} *)