Libguestfs - Feb 2016 - Re: [PATCH 3/4] v2v: take requested caps into account when converting

On Tue, Feb 09, 2016 at 05:53:57PM +0300, Roman Kagan
wrote:
> diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml
> index bdce038..8e0430c 100644
> --- a/v2v/windows_virtio.ml
> +++ b/v2v/windows_virtio.ml
> @@ -33,57 +33,105 @@ let virtio_win >      with Not_found ->
>        Guestfs_config.datadir // "virtio-win"
>  
> -let rec install_drivers g inspect systemroot root current_cs > +let rec
install_drivers g inspect systemroot root current_cs rcaps >    (* Copy the
virtio drivers to the guest. *)
>    let driverdir = sprintf "%s/Drivers/VirtIO" systemroot in
>    g#mkdir_p driverdir;
>  
>    if not (copy_drivers g inspect driverdir) then (
> +    let block_type = match rcaps.rcaps_block_bus with
> +                     | None -> IDE
> +                     | Some block_type -> block_type in
> +    let net_type = match rcaps.rcaps_net_bus with
> +                   | None -> RTL8139
> +                   | Some net_type -> net_type in
> +    let video = match rcaps.rcaps_video with
> +                | None -> Cirrus
> +                | Some video -> video in
> +
> +    if block_type = Virtio_blk || net_type = Virtio_net || video = QXL
then
> +      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;
This is a bit obscure, and type unsafe.

I think it's better to cover all the cases by a big match statement.
The code would look something like this:

  if not (copy_drivers g inspect driverdir) then (
    match rcaps with
    | { rcaps_block_bus = Some Virtio_blk }
    | { rcaps_net_bus = Some Virtio_net }
    | { rcaps_video = Some QXL } ->
       (* User requested virtio, but we have no virtio-win drivers. *)
       error [...]
    | { rcaps_block_bus = (Some IDE | None);
        rcaps_net_bus = ((Some E1000 | Some RTL8139 | None) as net_type);
        rcaps_video = (Some Cirrus | None) } ->
      warning [...];
      let net_type         match net_type with
        | Some model -> model
	| None -> RTL8139 in
      (IDE, model, Cirrus)
  )
  else (
    ...
>    else (
>      (* Can we install the block driver? *)
>      let block : guestcaps_block_type > -      let source = driverdir //
"viostor.sys" in
> -      if g#exists source then (
> +      let has_viostor = g#exists (driverdir // "viostor.inf") in
> +      match rcaps.rcaps_block_bus with
> +      | None ->
> +          if not has_viostor then (
> +            warning (f_"there is no viostor (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
> +          )
> +          else
> +            Virtio_blk
> +      | Some blk_type ->
> +          if blk_type = Virtio_blk && not has_viostor then
> +            error (f_"there is no viostor (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;
> +          blk_type
> +    in
> +
> +    (* Block driver needs tweaks to allow booting; the rest is set up by
PnP
> +     * manager *)
> +    if block = Virtio_blk then (
> +        let source = driverdir // "viostor.sys" in
>          let target = sprintf "%s/system32/drivers/viostor.sys"
systemroot in
>          let target = g#case_sensitive_path target in
>          g#cp source target;
> -        add_viostor_to_registry g inspect root current_cs;
> -        Virtio_blk
> -      ) else (
> -        warning (f_"there is no viostor (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
> -      ) in
> +        add_viostor_to_registry g inspect root current_cs
> +    );
Again, I found this code (and the following code for net/video) to be
very confusing.  Any time you've got multiple levels of match + if,
you're probably doing it wrong, and it's better to do the whole lot
with pattern matching (especially since the compiler helps you to find
missing cases this way).  For example:

  match rcaps.rcaps_block_bus, has_viostor with
  | { None, false }
    (* Warn the user things could be better with virtio-win, but continue
     * with IDE.
     *)
    warning [...];
    IDE

  | { Some IDE, false } ->
    (* User requested IDE, so no warning is required. *)
    IDE

  | { Some Virtio_blk, false } ->
    (* Error: request cannot be satisfied *)
    error [...];

  | { None, true } ->
    (* Good news, we can configure virtio-win. *)

  etc etc.

I'm going to have to think about this some more.

Rich.
>      (* Can we install the virtio-net driver? *)
>      let net : guestcaps_net_type > -      if not (g#exists (driverdir
// "netkvm.inf")) 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
> -      )
> -      else
> -        (* It will be installed at firstboot. *)
> -        Virtio_net in
> +      let has_netkvm = g#exists (driverdir // "netkvm.inf") in
> +      match rcaps.rcaps_net_bus with
> +      | None ->
> +          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
> +          )
> +          else
> +            Virtio_net
> +      | Some net_type ->
> +          if net_type = Virtio_net && not has_netkvm then
> +            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;
> +          net_type
> +    in
>  
>      (* Can we install the QXL driver? *)
>      let video : guestcaps_video_type > -      if not (g#exists
(driverdir // "qxl.inf")) 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 standard VGA.")
> -                inspect.i_major_version inspect.i_minor_version
> -                inspect.i_arch virtio_win;
> -        Cirrus
> -      )
> -      else
> -        (* It will be installed at firstboot. *)
> -        QXL in
> +      let has_qxl = g#exists (driverdir // "qxl.inf") in
> +      match rcaps.rcaps_video with
> +      | None ->
> +          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 standard VGA.")
> +                    inspect.i_major_version inspect.i_minor_version
> +                    inspect.i_arch virtio_win;
> +            Cirrus
> +          )
> +          else
> +            QXL
> +      | Some video ->
> +          if video = QXL && not has_qxl then
> +            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;
> +          video
> +    in
>  
>      (block, net, video)
>    )
> diff --git a/v2v/windows_virtio.mli b/v2v/windows_virtio.mli
> index eb7a57a..1b3f14a 100644
> --- a/v2v/windows_virtio.mli
> +++ b/v2v/windows_virtio.mli
> @@ -20,8 +20,9 @@
>  
>  val install_drivers
>      : Guestfs.guestfs -> Types.inspect -> string -> int64 ->
string ->
> +      Types.requested_guestcaps ->
>        Types.guestcaps_block_type * Types.guestcaps_net_type *
Types.guestcaps_video_type
> -(** [install_drivers g inspect systemroot virtio_win root current_cs]
> +(** [install_drivers g inspect systemroot virtio_win root current_cs
rcaps]
>      installs virtio drivers from the driver directory or driver
>      ISO ([virtio_win]) into the guest driver directory and updates
>      the registry so that the [viostor.sys] driver gets loaded by
> @@ -31,6 +32,11 @@ val install_drivers
>      when this function is called).  [current_cs] is the name of the
>      [CurrentControlSet] (eg. ["ControlSet001"]).
>  
> +    [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)]
>      reflecting what devices are now required by the guest, either
>      virtio devices if we managed to install those, or legacy devices
> -- 
> 2.5.0
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

On Tue, Feb 09, 2016 at 07:02:06PM +0000, Richard W.M. Jones
wrote:
>       let net_type >         match net_type with
>         | Some model -> model
> 	| None -> RTL8139 in
>       (IDE, model, Cirrus)
              ^^ should be net_type, not model

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

On Tue, Feb 09, 2016 at 07:02:06PM +0000, Richard W.M. Jones
wrote:
> On Tue, Feb 09, 2016 at 05:53:57PM +0300, Roman Kagan wrote:
> > diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml
> > index bdce038..8e0430c 100644
> > --- a/v2v/windows_virtio.ml
> > +++ b/v2v/windows_virtio.ml
> > @@ -33,57 +33,105 @@ let virtio_win > >      with Not_found
->
> >        Guestfs_config.datadir // "virtio-win"
> >  
> > -let rec install_drivers g inspect systemroot root current_cs >
> +let rec install_drivers g inspect systemroot root current_cs rcaps >
>    (* Copy the virtio drivers to the guest. *)
> >    let driverdir = sprintf "%s/Drivers/VirtIO" systemroot in
> >    g#mkdir_p driverdir;
> >  
> >    if not (copy_drivers g inspect driverdir) then (
> > +    let block_type = match rcaps.rcaps_block_bus with
> > +                     | None -> IDE
> > +                     | Some block_type -> block_type in
> > +    let net_type = match rcaps.rcaps_net_bus with
> > +                   | None -> RTL8139
> > +                   | Some net_type -> net_type in
> > +    let video = match rcaps.rcaps_video with
> > +                | None -> Cirrus
> > +                | Some video -> video in
> > +
> > +    if block_type = Virtio_blk || net_type = Virtio_net || video =
QXL then
> > +      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;
> 
> This is a bit obscure, and type unsafe.
> 
> I think it's better to cover all the cases by a big match statement.
> The code would look something like this:
> 
>   if not (copy_drivers g inspect driverdir) then (
>     match rcaps with
>     | { rcaps_block_bus = Some Virtio_blk }
>     | { rcaps_net_bus = Some Virtio_net }
>     | { rcaps_video = Some QXL } ->
>        (* User requested virtio, but we have no virtio-win drivers. *)
>        error [...]
>     | { rcaps_block_bus = (Some IDE | None);
>         rcaps_net_bus = ((Some E1000 | Some RTL8139 | None) as net_type);
>         rcaps_video = (Some Cirrus | None) } ->
>       warning [...];
>       let net_type >         match net_type with
>         | Some model -> model
> 	| None -> RTL8139 in
>       (IDE, model, Cirrus)
>   )
>   else (
>     ...
> 
> >    else (
> >      (* Can we install the block driver? *)
> >      let block : guestcaps_block_type > > -      let source =
driverdir // "viostor.sys" in
> > -      if g#exists source then (
> > +      let has_viostor = g#exists (driverdir //
"viostor.inf") in
> > +      match rcaps.rcaps_block_bus with
> > +      | None ->
> > +          if not has_viostor then (
> > +            warning (f_"there is no viostor (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
> > +          )
> > +          else
> > +            Virtio_blk
> > +      | Some blk_type ->
> > +          if blk_type = Virtio_blk && not has_viostor then
> > +            error (f_"there is no viostor (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;
> > +          blk_type
> > +    in
> > +
> > +    (* Block driver needs tweaks to allow booting; the rest is set up
by PnP
> > +     * manager *)
> > +    if block = Virtio_blk then (
> > +        let source = driverdir // "viostor.sys" in
> >          let target = sprintf
"%s/system32/drivers/viostor.sys" systemroot in
> >          let target = g#case_sensitive_path target in
> >          g#cp source target;
> > -        add_viostor_to_registry g inspect root current_cs;
> > -        Virtio_blk
> > -      ) else (
> > -        warning (f_"there is no viostor (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
> > -      ) in
> > +        add_viostor_to_registry g inspect root current_cs
> > +    );
> 
> Again, I found this code (and the following code for net/video) to be
> very confusing.  Any time you've got multiple levels of match + if,
> you're probably doing it wrong, and it's better to do the whole lot
> with pattern matching (especially since the compiler helps you to find
> missing cases this way).  For example:
> 
>   match rcaps.rcaps_block_bus, has_viostor with
>   | { None, false }
>     (* Warn the user things could be better with virtio-win, but continue
>      * with IDE.
>      *)
>     warning [...];
>     IDE
> 
>   | { Some IDE, false } ->
>     (* User requested IDE, so no warning is required. *)
>     IDE
> 
>   | { Some Virtio_blk, false } ->
>     (* Error: request cannot be satisfied *)
>     error [...];
> 
>   | { None, true } ->
>     (* Good news, we can configure virtio-win. *)
> 
>   etc etc.
> 
> I'm going to have to think about this some more.
OK while you're still at it, let me add a bit to this discussion, as
I'm not happy about this patchset, too, because it doesn't fit well with
the overall existing v2v structure.

I think v2v in general falls into the following steps:

1) collecting information about the source VM (config, r/o inspection)

2) deciding on the target VM configuration, depending on the data from
   step (1) and on the availability of the necessary stuff like virtio
   drivers, etc.

3) creating the target VM with overlays over original disks

4) tuning up the guest to work in the new virtual hardware (installing
   drivers, fixing up boot, etc.)

5) finalizing the VM's disks


Note that

- step (1) may be done by another tool which has better knowledge of the
  source hypervisor (especially in the case of proprietary hypervisors)

- step (2) may be done with user interaction, e.g. in a UI.  Besides,
  the user may have reasons not to switch to virtio even when it's
  supported, or only do so for some of the VM's disks, or prefer
  virtio-scsi over virtio-blk, etc.

- step (4) is not supposed to take any decisions, but follow precisely
  the VM configuration and fail if it can't.  This makes up for a
  sensible self-contained tool; one of its use cases apart from v2v
  would be adjusting the guest to virtual hardware changes done by the
  user on an existing VM, without migrating to another hypervisor (e.g.
  switching from IDE to virtio-blk or from virtio-blk to virtio-scsi).
  [This was approximately what I was trying to achieve with --in-place
  but apparently failed so far.]

- step (5) may take different forms, e.g. the user may want to start
  using the VM immediately and merge the data from the original images
  in the background, or just leave them as backing images for good, or
  do an explicit copy to the final images as is currently done

Does this make sense?  If so, what would be the best way to proceed?
With my Virtuozzo hat on I'm interested only in step (4) because other
steps are easier to do in our own toolset; however I'm willing to do the
legwork to keep the things consistent.

Thanks,
Roman.

On Mon, Feb 15, 2016 at 08:21:11PM +0300, Roman Kagan
wrote:
> On Tue, Feb 09, 2016 at 07:02:06PM +0000, Richard W.M. Jones wrote:
> > On Tue, Feb 09, 2016 at 05:53:57PM +0300, Roman Kagan wrote:
> > > diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml
> > > index bdce038..8e0430c 100644
> > > --- a/v2v/windows_virtio.ml
> > > +++ b/v2v/windows_virtio.ml
> > > @@ -33,57 +33,105 @@ let virtio_win > > >      with
Not_found ->
> > >        Guestfs_config.datadir // "virtio-win"
> > >  
> > > -let rec install_drivers g inspect systemroot root current_cs
> > > +let rec install_drivers g inspect systemroot root current_cs
rcaps > > >    (* Copy the virtio drivers to the guest. *)
> > >    let driverdir = sprintf "%s/Drivers/VirtIO"
systemroot in
> > >    g#mkdir_p driverdir;
> > >  
> > >    if not (copy_drivers g inspect driverdir) then (
> > > +    let block_type = match rcaps.rcaps_block_bus with
> > > +                     | None -> IDE
> > > +                     | Some block_type -> block_type in
> > > +    let net_type = match rcaps.rcaps_net_bus with
> > > +                   | None -> RTL8139
> > > +                   | Some net_type -> net_type in
> > > +    let video = match rcaps.rcaps_video with
> > > +                | None -> Cirrus
> > > +                | Some video -> video in
> > > +
> > > +    if block_type = Virtio_blk || net_type = Virtio_net || video
= QXL then
> > > +      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;
> > 
> > This is a bit obscure, and type unsafe.
> > 
> > I think it's better to cover all the cases by a big match
statement.
> > The code would look something like this:
> > 
> >   if not (copy_drivers g inspect driverdir) then (
> >     match rcaps with
> >     | { rcaps_block_bus = Some Virtio_blk }
> >     | { rcaps_net_bus = Some Virtio_net }
> >     | { rcaps_video = Some QXL } ->
> >        (* User requested virtio, but we have no virtio-win drivers. *)
> >        error [...]
> >     | { rcaps_block_bus = (Some IDE | None);
> >         rcaps_net_bus = ((Some E1000 | Some RTL8139 | None) as
net_type);
> >         rcaps_video = (Some Cirrus | None) } ->
> >       warning [...];
> >       let net_type > >         match net_type with
> >         | Some model -> model
> > 	| None -> RTL8139 in
> >       (IDE, model, Cirrus)
> >   )
> >   else (
> >     ...
> > 
> > >    else (
> > >      (* Can we install the block driver? *)
> > >      let block : guestcaps_block_type > > > -      let
source = driverdir // "viostor.sys" in
> > > -      if g#exists source then (
> > > +      let has_viostor = g#exists (driverdir //
"viostor.inf") in
> > > +      match rcaps.rcaps_block_bus with
> > > +      | None ->
> > > +          if not has_viostor then (
> > > +            warning (f_"there is no viostor (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
> > > +          )
> > > +          else
> > > +            Virtio_blk
> > > +      | Some blk_type ->
> > > +          if blk_type = Virtio_blk && not has_viostor
then
> > > +            error (f_"there is no viostor (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;
> > > +          blk_type
> > > +    in
> > > +
> > > +    (* Block driver needs tweaks to allow booting; the rest is
set up by PnP
> > > +     * manager *)
> > > +    if block = Virtio_blk then (
> > > +        let source = driverdir // "viostor.sys" in
> > >          let target = sprintf
"%s/system32/drivers/viostor.sys" systemroot in
> > >          let target = g#case_sensitive_path target in
> > >          g#cp source target;
> > > -        add_viostor_to_registry g inspect root current_cs;
> > > -        Virtio_blk
> > > -      ) else (
> > > -        warning (f_"there is no viostor (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
> > > -      ) in
> > > +        add_viostor_to_registry g inspect root current_cs
> > > +    );
> > 
> > Again, I found this code (and the following code for net/video) to be
> > very confusing.  Any time you've got multiple levels of match +
if,
> > you're probably doing it wrong, and it's better to do the
whole lot
> > with pattern matching (especially since the compiler helps you to find
> > missing cases this way).  For example:
> > 
> >   match rcaps.rcaps_block_bus, has_viostor with
> >   | { None, false }
> >     (* Warn the user things could be better with virtio-win, but
continue
> >      * with IDE.
> >      *)
> >     warning [...];
> >     IDE
> > 
> >   | { Some IDE, false } ->
> >     (* User requested IDE, so no warning is required. *)
> >     IDE
> > 
> >   | { Some Virtio_blk, false } ->
> >     (* Error: request cannot be satisfied *)
> >     error [...];
> > 
> >   | { None, true } ->
> >     (* Good news, we can configure virtio-win. *)
> > 
> >   etc etc.
> > 
> > I'm going to have to think about this some more.
[Actually I wasn't still thinking about it ...]
> OK while you're still at it, let me add a bit to this discussion, as
> I'm not happy about this patchset, too, because it doesn't fit well
with
> the overall existing v2v structure.
> 
> I think v2v in general falls into the following steps:
> 
> 1) collecting information about the source VM (config, r/o inspection)
> 
> 2) deciding on the target VM configuration, depending on the data from
>    step (1) and on the availability of the necessary stuff like virtio
>    drivers, etc.
> 
> 3) creating the target VM with overlays over original disks
> 
> 4) tuning up the guest to work in the new virtual hardware (installing
>    drivers, fixing up boot, etc.)
> 
> 5) finalizing the VM's disks
> 
> 
> Note that
> 
> - step (1) may be done by another tool which has better knowledge of the
>   source hypervisor (especially in the case of proprietary hypervisors)
> 
> - step (2) may be done with user interaction, e.g. in a UI.  Besides,
>   the user may have reasons not to switch to virtio even when it's
>   supported, or only do so for some of the VM's disks, or prefer
>   virtio-scsi over virtio-blk, etc.
> 
> - step (4) is not supposed to take any decisions, but follow precisely
>   the VM configuration and fail if it can't.  This makes up for a
>   sensible self-contained tool; one of its use cases apart from v2v
>   would be adjusting the guest to virtual hardware changes done by the
>   user on an existing VM, without migrating to another hypervisor (e.g.
>   switching from IDE to virtio-blk or from virtio-blk to virtio-scsi).
>   [This was approximately what I was trying to achieve with --in-place
>   but apparently failed so far.]
For our use case, we don't really want lots of user adjustable
settings, since we are trying to import 1000s of VMs in one go.
However that doesn't mean you can't add customization flags to the
command line for your use case.
> - step (5) may take different forms, e.g. the user may want to start
>   using the VM immediately and merge the data from the original images
>   in the background, or just leave them as backing images for good, or
>   do an explicit copy to the final images as is currently done
> 
> Does this make sense?  If so, what would be the best way to proceed?
A reasonable step forward now would be to repost the patch set,
rebased on top of current master, and with the changes I outlined in
the reviews of 1/4 and 3/4 (I didn't look at 4/4 yet).
> With my Virtuozzo hat on I'm interested only in step (4) because other
> steps are easier to do in our own toolset; however I'm willing to do
the
> legwork to keep the things consistent.
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