[Libguestfs] [PATCH 3/4] v2v: take requested caps into account when converting
Richard W.M. Jones
rjones at redhat.com
Mon Feb 15 19:11:05 UTC 2016
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
More information about the Libguestfs
mailing list