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