[Libguestfs] [PATCH v3] v2v: add support for virtio-scsi

[Richard W.M. Jones]

On Thu, Apr 14, 2016 at 12:01:30PM +0300, Roman Kagan wrote:
> @@ -1134,7 +1135,13 @@ 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 ->
> +      let block_module =
> +        match block_type with
> +        | Virtio_blk -> "virtio_blk"
> +        | Virtio_SCSI -> "virtio_scsi"
> +        | _ -> raise Not_found in

It's a bad idea to raise Not_found unless you're going to catch it
immediately.  As this is an impossible condition you should use
`assert false' here.

> @@ -105,6 +106,10 @@ object
>            )
>          ] in
>          let properties =
> +          match guestcaps.gcaps_block_bus with
> +          | Virtio_SCSI -> ("hw_scsi_model", "virtio-scsi") :: properties
> +          | _ -> properties in

Since there are only two other cases, it's better to match them
explicitly, ie:

  | Virtio_blk | IDE -> properties in

The reason for this is if we add another block type in future the
compiler will error out because of the missing case, forcing us to
analyze whether we need to add some other property for it.

> 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.")

As above, use `| IDE ->' instead of this wildcard match, so that in
future we are forced to analyze this code if we add another block
type.

> +      | 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.")

'passthrough'?  I know that the virtio SCSI driver passes SCSI
commands through, but I think this might be confused with PCI / host
adapter passthrough which is a completely different thing.  Probably
better to drop this word.

-----

ACK if you make all of the changes above.

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/