[Libguestfs] [PATCH 1/4] v2v: collect source network and video adapter types

[Richard W.M. Jones]

On Tue, Feb 09, 2016 at 05:53:55PM +0300, Roman Kagan wrote:
> +and s_nic_model =  Source_rtl8139 | Source_e1000 | Source_virtio_net
[...]
> +and source_video = Source_Cirrus | Source_QXL

As you know there could be a lot of other input devices.

At the moment the code basically ignores these (but it prints a
warning "unknown [video|network] adapter model %s ignored").  I think
we should only print warnings when that information is useful and/or
actionable for the user, which it isn't in this case.  (I know we
don't always obey that rule right now, but I think it's still a good
rule ...)

So I think a better way to do it would be:

  and s_nic_model =
  | Source_rtl8139 | Source_e1000 | Source_virtio_net
  | Source_other_nic of string

  and source_video =
  | Source_Cirrus | Source_QXL
  | Source_other_video of string

and then change the match code to be something like:

  let model =
    match xpath_string "model/@type" with
    | None -> None
    | Some "virtio" -> Some Source_virtio_net
    | Some "e1000" -> Some Source_e1000
    | Some "rtl8139" -> Some Source_rtl8139
    | Some model -> Some (Source_other_nic model) in

(similarly for video model).  We don't throw away the NIC model if
it's something we can't handle, but we still have special cases for
virtio-net/e1000/rtl8139, so we can handle them in a type safe way.

BTW if you're wondering why I sometimes put the 'in' of 'let .. in' on
the right hand side of the last line of code, and sometimes on a line
by itself, it's because the rule is: Attach 'in' to the last line if
what you're defining is a non-function.  Put 'in' on a line on its own
if what you're defining is a function.  Above is an example of a
non-function.  An example of a function definition would be:

  let f () =
    ...
  in

Apart from that, the patch looks fine to me.

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