Virtio-SCSI offers a number of advantages over virtio-blk, in
particular, it supports SCSI UNMAP (aka trim) which is crucial for
keeping the virtual drive from wasting host disk space.
This patch adds support for virtio-scsi as the virtual disk connection
type both on input and on output of v2v.
Virtio-blk remains the default, so for now virtio-scsi-based guests can
only be produced in --in-place mode.
Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
v2v/OVF.ml | 4 +++-
v2v/convert_linux.ml | 15 +++++++++------
v2v/input_libvirtxml.ml | 32 ++++++++++++++++++++------------
v2v/output_glance.ml | 1 +
v2v/test-v2v-print-source.sh | 2 +-
v2v/types.ml | 9 ++++++---
v2v/types.mli | 14 +++++++-------
v2v/v2v.ml | 13 ++++++++-----
v2v/windows_virtio.ml | 30 ++++++++++++++++++++++++------
9 files changed, 79 insertions(+), 41 deletions(-)
diff --git a/v2v/OVF.ml b/v2v/OVF.ml
index f8afc8c..dc1e971 100644
--- a/v2v/OVF.ml
+++ b/v2v/OVF.ml
@@ -454,7 +454,9 @@ and add_disks targets guestcaps output_alloc sd_uuid
image_uuids vol_uuids ovf "ovf:format",
"http://en.wikipedia.org/wiki/Byte"; (* wtf? *)
"ovf:disk-interface",
(match guestcaps.gcaps_block_bus with
- | Virtio_blk -> "VirtIO" | IDE -> "IDE");
+ | Virtio_blk -> "VirtIO"
+ | Virtio_SCSI -> "SCSI"
+ | IDE -> "IDE");
"ovf:disk-type", "System"; (* RHBZ#744538 *)
"ovf:boot", if is_boot_drive then "True" else
"False";
] in
diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
index 0a2e439..90355fb 100644
--- a/v2v/convert_linux.ml
+++ b/v2v/convert_linux.ml
@@ -803,8 +803,9 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect
source rcaps * major number of vdX block devices. If we change it,
RHEL 3
* KVM guests won't boot.
*)
- [ "virtio"; "virtio_ring";
"virtio_blk"; "virtio_net";
- "virtio_pci" ]
+ List.filter (fun m -> List.mem m kernel.ki_modules)
+ [ "virtio"; "virtio_ring";
"virtio_blk";
+ "virtio_scsi"; "virtio_net";
"virtio_pci" ]
else
[ "sym53c8xx" (* XXX why not "ide"? *) ] in
@@ -1134,7 +1135,7 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps (* Update 'alias scsi_hostadapter ...' *)
let paths = augeas_modprobe ". =~
regexp('scsi_hostadapter.*')" in
match block_type with
- | Virtio_blk ->
+ | Virtio_blk | Virtio_SCSI ->
if paths <> [] then (
(* There's only 1 scsi controller in the converted guest.
* Convert only the first scsi_hostadapter entry to virtio
@@ -1148,14 +1149,15 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps List.iter (fun path -> ignore (g#aug_rm path))
(List.rev paths_to_delete);
- g#aug_set (path ^ "/modulename") "virtio_blk"
+ g#aug_set (path ^ "/modulename")
+ (string_of_block_type block_type)
) else (
(* We have to add a scsi_hostadapter. *)
let modpath = discover_modpath () in
g#aug_set (sprintf "/files%s/alias[last()+1]" modpath)
"scsi_hostadapter";
g#aug_set (sprintf "/files%s/alias[last()]/modulename"
modpath)
- "virtio_blk"
+ (string_of_block_type block_type)
)
| IDE ->
(* There is no scsi controller in an IDE guest. *)
@@ -1247,6 +1249,7 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps let block_prefix_after_conversion match
block_type with
| Virtio_blk -> "vd"
+ | Virtio_SCSI -> "sd"
| IDE -> ide_block_prefix in
let map @@ -1255,7 +1258,7 @@ let rec convert ~keep_serial_console (g :
G.guestfs) inspect source rcaps let block_prefix_before_conversion
match disk.s_controller with
| Some Source_IDE -> ide_block_prefix
- | Some Source_SCSI -> "sd"
+ | Some Source_virtio_SCSI | Some Source_SCSI -> "sd"
| Some Source_virtio_blk -> "vd"
| None ->
(* This is basically a guess. It assumes the source used IDE. *)
diff --git a/v2v/input_libvirtxml.ml b/v2v/input_libvirtxml.ml
index 9d8963d..6f325e6 100644
--- a/v2v/input_libvirtxml.ml
+++ b/v2v/input_libvirtxml.ml
@@ -181,6 +181,12 @@ let parse_libvirt_xml ?conn xml None
) in
+ (* Presence of virtio-scsi controller. *)
+ let has_virtio_scsi + let obj = Xml.xpath_eval_expression xpathctx
+
"/domain/devices/controller[@model='virtio-scsi']" in
+ (Xml.xpathobj_nr_nodes obj) > 0 in
+
(* Non-removable disk devices. *)
let disks let get_disks, add_disk @@ -208,12 +214,13 @@ let
parse_libvirt_xml ?conn xml
let controller let target_bus = xpath_string
"target/@bus" in
- match target_bus with
- | None -> None
- | Some "ide" -> Some Source_IDE
- | Some "scsi" -> Some Source_SCSI
- | Some "virtio" -> Some Source_virtio_blk
- | Some _ -> None in
+ match target_bus, has_virtio_scsi with
+ | None, _ -> None
+ | Some "ide", _ -> Some Source_IDE
+ | Some "scsi", true -> Some Source_virtio_SCSI
+ | Some "scsi", false -> Some Source_SCSI
+ | Some "virtio", _ -> Some Source_virtio_blk
+ | Some _, _ -> None in
let format match xpath_string "driver/@type" with
@@ -297,12 +304,13 @@ let parse_libvirt_xml ?conn xml
let controller let target_bus = xpath_string
"target/@bus" in
- match target_bus with
- | None -> None
- | Some "ide" -> Some Source_IDE
- | Some "scsi" -> Some Source_SCSI
- | Some "virtio" -> Some Source_virtio_blk
- | Some _ -> None in
+ match target_bus, has_virtio_scsi with
+ | None, _ -> None
+ | Some "ide", _ -> Some Source_IDE
+ | Some "scsi", true -> Some Source_virtio_SCSI
+ | Some "scsi", false -> Some Source_SCSI
+ | Some "virtio", _ -> Some Source_virtio_blk
+ | Some _, _ -> None in
let slot let target_dev = xpath_string "target/@dev"
in
diff --git a/v2v/output_glance.ml b/v2v/output_glance.ml
index 9487f0b..2572de6 100644
--- a/v2v/output_glance.ml
+++ b/v2v/output_glance.ml
@@ -86,6 +86,7 @@ object
"hw_disk_bus",
(match guestcaps.gcaps_block_bus with
| Virtio_blk -> "virtio"
+ | Virtio_SCSI -> "scsi"
| IDE -> "ide");
"hw_vif_model",
(match guestcaps.gcaps_net_bus with
diff --git a/v2v/test-v2v-print-source.sh b/v2v/test-v2v-print-source.sh
index 8af6104..789555d 100755
--- a/v2v/test-v2v-print-source.sh
+++ b/v2v/test-v2v-print-source.sh
@@ -63,7 +63,7 @@ hypervisor type: test
video: qxl
sound:
disks:
- /windows.img (raw) [virtio]
+ /windows.img (raw) [virtio-blk]
removable media:
NICs:
Network \"default\" mac: 00:11:22:33:44:55 [virtio]" ]; then
diff --git a/v2v/types.ml b/v2v/types.ml
index 821b7ec..9a23b4a 100644
--- a/v2v/types.ml
+++ b/v2v/types.ml
@@ -55,7 +55,8 @@ and source_disk = {
s_format : string option;
s_controller : s_controller option;
}
-and s_controller = Source_IDE | Source_SCSI | Source_virtio_blk
+and s_controller = Source_IDE | Source_SCSI |
+ Source_virtio_blk | Source_virtio_SCSI
and source_removable = {
s_removable_type : s_removable_type;
s_removable_controller : s_controller option;
@@ -187,7 +188,8 @@ and string_of_source_disk { s_qemu_uri = qemu_uri; s_format
= format;
and string_of_controller = function
| Source_IDE -> "ide"
| Source_SCSI -> "scsi"
- | Source_virtio_blk -> "virtio"
+ | Source_virtio_blk -> "virtio-blk"
+ | Source_virtio_SCSI -> "virtio-scsi"
and string_of_source_removable { s_removable_type = typ;
s_removable_controller = controller;
@@ -366,13 +368,14 @@ and requested_guestcaps = {
rcaps_net_bus : guestcaps_net_type option;
rcaps_video : guestcaps_video_type option;
}
-and guestcaps_block_type = Virtio_blk | IDE
+and guestcaps_block_type = Virtio_blk | Virtio_SCSI | IDE
and guestcaps_net_type = Virtio_net | E1000 | RTL8139
and guestcaps_video_type = QXL | Cirrus
let string_of_block_type block_type (match block_type with
| Virtio_blk -> "virtio-blk"
+ | Virtio_SCSI -> "virtio-scsi"
| IDE -> "ide")
let string_of_net_type net_type (match net_type with
diff --git a/v2v/types.mli b/v2v/types.mli
index 4595a1f..71cfa31 100644
--- a/v2v/types.mli
+++ b/v2v/types.mli
@@ -65,12 +65,9 @@ and source_disk = {
}
(** A source disk. *)
-and s_controller = Source_IDE | Source_SCSI | Source_virtio_blk
-(** Source disk controller.
-
- For the purposes of this field, we can treat virtio-scsi as
- [SCSI]. However we don't support conversions from virtio in any
- case so virtio is here only to make it work for testing. *)
+and s_controller = Source_IDE | Source_SCSI |
+ Source_virtio_blk | Source_virtio_SCSI
+(** Source disk controller. *)
and source_removable = {
s_removable_type : s_removable_type; (** Type. *)
@@ -218,10 +215,13 @@ and requested_guestcaps = {
}
(** Guest capabilities after conversion. eg. Was virtio found or installed? *)
-and guestcaps_block_type = Virtio_blk | IDE
+and guestcaps_block_type = Virtio_blk | Virtio_SCSI | IDE
and guestcaps_net_type = Virtio_net | E1000 | RTL8139
and guestcaps_video_type = QXL | Cirrus
+val string_of_block_type : guestcaps_block_type -> string
+val string_of_net_type : guestcaps_net_type -> string
+val string_of_video : guestcaps_video_type -> string
val string_of_guestcaps : guestcaps -> string
val string_of_requested_guestcaps : requested_guestcaps -> string
diff --git a/v2v/v2v.ml b/v2v/v2v.ml
index 4d0d525..647ae11 100644
--- a/v2v/v2v.ml
+++ b/v2v/v2v.ml
@@ -731,10 +731,11 @@ and do_convert g inspect source keep_serial_console rcaps
(* Did we manage to install virtio drivers? *)
if not (quiet ()) then (
- if guestcaps.gcaps_block_bus = Virtio_blk then
- info (f_"This guest has virtio drivers installed.")
- else
- info (f_"This guest does not have virtio drivers installed.");
+ match guestcaps.gcaps_block_bus with
+ | Virtio_blk | Virtio_SCSI ->
+ info (f_"This guest has virtio drivers installed.")
+ | _ ->
+ info (f_"This guest does not have virtio drivers installed.")
);
guestcaps
@@ -939,6 +940,7 @@ and target_bus_assignment source targets guestcaps
let t = BusSlotTarget t in
match guestcaps.gcaps_block_bus with
| Virtio_blk -> insert virtio_blk_bus i t
+ | Virtio_SCSI -> insert scsi_bus i t
| IDE -> insert ide_bus i t
) targets;
@@ -952,7 +954,7 @@ and target_bus_assignment source targets guestcaps
| None -> ide_bus (* Wild guess, but should be safe. *)
| Some Source_virtio_blk -> virtio_blk_bus
| Some Source_IDE -> ide_bus
- | Some Source_SCSI -> scsi_bus in
+ | Some Source_virtio_SCSI | Some Source_SCSI -> scsi_bus in
match r.s_removable_slot with
| None -> ignore (insert_after bus 0 (BusSlotRemovable r))
| Some desired_slot_nr ->
@@ -992,6 +994,7 @@ and rcaps_from_source source let block_type match
source_block_type with
| Some Source_virtio_blk -> Some Virtio_blk
+ | Some Source_virtio_SCSI -> Some Virtio_SCSI
| Some Source_IDE -> Some IDE
| Some t -> error (f_"source has unsupported hard disk type
'%s'")
(string_of_controller t)
diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml
index 7ca4068..a54dbca 100644
--- a/v2v/windows_virtio.ml
+++ b/v2v/windows_virtio.ml
@@ -35,6 +35,7 @@ let virtio_win
let scsi_class_guid = "{4D36E97B-E325-11CE-BFC1-08002BE10318}"
let viostor_pciid =
"VEN_1AF4&DEV_1001&SUBSYS_00021AF4&REV_00"
+let vioscsi_pciid =
"VEN_1AF4&DEV_1004&SUBSYS_00081AF4&REV_00"
let rec install_drivers g inspect systemroot root current_cs rcaps (* Copy
the virtio drivers to the guest. *)
@@ -43,7 +44,7 @@ let rec install_drivers g inspect systemroot root current_cs
rcaps
if not (copy_drivers g inspect driverdir) then (
match rcaps with
- | { rcaps_block_bus = Some Virtio_blk }
+ | { 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")
@@ -66,19 +67,25 @@ let rec install_drivers g inspect systemroot root current_cs
rcaps (* Can we install the block driver? *)
let block : guestcaps_block_type let has_viostor = g#exists
(driverdir // "viostor.inf") in
- match rcaps.rcaps_block_bus, has_viostor with
- | Some Virtio_blk, false ->
+ let has_vioscsi = g#exists (driverdir // "vioscsi.inf") in
+ match rcaps.rcaps_block_bus, has_viostor, has_vioscsi with
+ | Some Virtio_blk, false, _ ->
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
- | None, false ->
+ | Some Virtio_SCSI, _, false ->
+ error (f_"there is no vioscsi (virtio SCSI passthrough) 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, false, _ ->
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
- | (Some Virtio_blk | None), true ->
+ | (Some Virtio_blk | None), true, _ ->
(* Block driver needs tweaks to allow booting; the rest is set up by
PnP
* manager *)
let source = driverdir // "viostor.sys" in
@@ -89,7 +96,18 @@ let rec install_drivers g inspect systemroot root current_cs
rcaps viostor_pciid;
Virtio_blk
- | Some IDE, _ ->
+ | 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"
systemroot in
+ let target = g#case_sensitive_path target in
+ g#cp source target;
+ add_guestor_to_registry g root current_cs "vioscsi"
+ vioscsi_pciid;
+ Virtio_SCSI
+
+ | Some IDE, _, _ ->
IDE in
(* Can we install the virtio-net driver? *)
--
2.5.5
Richard W.M. Jones
2016-Apr-13 09:11 UTC
Re: [Libguestfs] [PATCH] v2v: add support for virtio-scsi
On Tue, Apr 12, 2016 at 06:46:31PM +0300, Roman Kagan wrote:> (match guestcaps.gcaps_block_bus with > - | Virtio_blk -> "VirtIO" | IDE -> "IDE"); > + | Virtio_blk -> "VirtIO" > + | Virtio_SCSI -> "SCSI"I believe this should be "VirtIO_SCSI", although it's difficult to get a straight answer from the massive and convoluted Java sources of ovirt-engine ...> @@ -1148,14 +1149,15 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect source rcaps > List.iter (fun path -> ignore (g#aug_rm path)) > (List.rev paths_to_delete); > > - g#aug_set (path ^ "/modulename") "virtio_blk" > + g#aug_set (path ^ "/modulename") > + (string_of_block_type block_type)Here ...> ) else ( > (* We have to add a scsi_hostadapter. *) > let modpath = discover_modpath () in > g#aug_set (sprintf "/files%s/alias[last()+1]" modpath) > "scsi_hostadapter"; > g#aug_set (sprintf "/files%s/alias[last()]/modulename" modpath) > - "virtio_blk" > + (string_of_block_type block_type) > )... and here don't seem right. Firstly, string_of_block_type returns "virtio-blk" (not "virtio_blk"). Maybe for this configuration file it doesn't matter. But, the real problem is that string_of_block_type is essentially an internal debugging function. What we should really have is a new function (eg. "linux_module_of_block_type") which converts the block type to a Linux module name.> + (* Presence of virtio-scsi controller. *) > + let has_virtio_scsi > + let obj = Xml.xpath_eval_expression xpathctx > + "/domain/devices/controller[@model='virtio-scsi']" inI guess this short cut is OK. A true test would involve checking the <target bus="scsi"> on each disk and matching it back to the controller. In other words, a huge pain! Maybe you can add an "XXX" comment in the source about this.> + (Xml.xpathobj_nr_nodes obj) > 0 in^ ^ You don't need these parentheses. In functional languages, function application always binds tightest of all, eg: f x + 1 is the same as: (f x) + 1> diff --git a/v2v/output_glance.ml b/v2v/output_glance.ml > index 9487f0b..2572de6 100644 > --- a/v2v/output_glance.ml > +++ b/v2v/output_glance.ml > @@ -86,6 +86,7 @@ object > "hw_disk_bus", > (match guestcaps.gcaps_block_bus with > | Virtio_blk -> "virtio" > + | Virtio_SCSI -> "scsi" > | IDE -> "ide"); > "hw_vif_model", > (match guestcaps.gcaps_net_bus withThis is wrong, or at least, incomplete. Apparently you need: hw_disk_bus=scsi (as above) hw_scsi_model=virtio-scsi (additional property) See: http://www.sebastien-han.fr/blog/2015/02/02/openstack-and-ceph-rbd-discard/> if not (copy_drivers g inspect driverdir) then ( > match rcaps with > - | { rcaps_block_bus = Some Virtio_blk } > + | { rcaps_block_bus = (Some Virtio_blk | Some Virtio_SCSI) }I don't think you need parens there. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Richard W.M. Jones
2016-Apr-13 17:39 UTC
Re: [Libguestfs] [PATCH] v2v: add support for virtio-scsi
On Wed, Apr 13, 2016 at 07:45:42PM +0300, Roman Kagan wrote:> On Wed, Apr 13, 2016 at 10:11:54AM +0100, Richard W.M. Jones wrote: > > On Tue, Apr 12, 2016 at 06:46:31PM +0300, Roman Kagan wrote: > > > + (* Presence of virtio-scsi controller. *) > > > + let has_virtio_scsi > > > + let obj = Xml.xpath_eval_expression xpathctx > > > + "/domain/devices/controller[@model='virtio-scsi']" in > > > > I guess this short cut is OK. A true test would involve checking the > > <target bus="scsi"> on each disk and matching it back to the > > controller. In other words, a huge pain! Maybe you can add an "XXX" > > comment in the source about this. > > On a second look, I'm not sure I got your comment right. > > AFAICS there's no way in libvirt xml to define multiple scsi buses,I think this is possible, although massively unlikely in practice. I think the different controllers would have different index="0"/index="1"/... attributes.> so all the matching I can do is just to notice the presence of > virtio-scsi controller, and attribute all SCSI drives to it. This > is exactly what happens in the patch. > > Am I missing something?So first off I think this part of the patch and your assumption is fine, there's no need to change the code. However there is a possibility that the source guest might have multiple mixed virtio-blk + virtio-scsi disks. In that case the disks would look like this: <disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file=...> <target dev='vda' bus='virtio'/> </disk> <disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file=...> <target dev='sda' bus='scsi'/> </disk> <controller type="scsi" index="0" model="virtio-scsi"/> It's not something that we should worry about, and a simple comment with "XXX" in it is fine, if you agree with my analysis. 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
Richard W.M. Jones
2016-Apr-13 18:32 UTC
Re: [Libguestfs] [PATCH] v2v: add support for virtio-scsi
On Wed, Apr 13, 2016 at 09:04:54PM +0300, Roman Kagan wrote:> On Wed, Apr 13, 2016 at 06:39:55PM +0100, Richard W.M. Jones wrote: > > I think this is possible, although massively unlikely in practice. I > > think the different controllers would have different > > index="0"/index="1"/... attributes. > > How the binding of drives to controllers is done, then? Is it specified > in drive's "address"?Yes, apparently using <address/> under <disk/>.> This case is handled fine in the patch, isn't it? has_virtio_scsi is a > local variable which is only consulted when populating per-drive > s_controller field: for drives with bus='scsi' it differentiates between > Source_virtio_SCSI and Source_SCSI.Oh I see, target_bus is consulted for each drive. So this is right (except for the scsi/virtio-scsi case, but who knows about that).> The place where we make up a single disk type is > v2v.ml:rcaps_from_source, but there I explicitly error out if the guest > has multiple disk types (which may be too harsh but simplifies things > for now).Yup. Rich. -- 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/