[Libguestfs] [PATCH v2 4/4] v2v: in-place: request caps based on source config

[Richard W.M. Jones]

On Sat, Feb 20, 2016 at 11:26:10AM +0300, Roman Kagan wrote:
> In in-place mode, the decisions on which interfaces to use are made and
> the source configuration is created by the outside entity.  So in that
> case v2v needs to look it up in the source configuraion, and try to
> follow.
> 
> For that, the source configuration is used to populate requested caps
> object which is then passed to the convert routine.
> 
> Signed-off-by: Roman Kagan <rkagan at virtuozzo.com>
> ---
> 
> Notes:
>     v2:
>      - accept catch-all variants of source net and video as no preference
> 
>  v2v/types.mli |  6 +++---
>  v2v/v2v.ml    | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 53 insertions(+), 8 deletions(-)
> 
> diff --git a/v2v/types.mli b/v2v/types.mli
> index 0e40668..fbd45cf 100644
> --- a/v2v/types.mli
> +++ b/v2v/types.mli
> @@ -66,7 +66,7 @@ and source_disk = {
>  (** A source disk. *)
>  
>  and s_controller = Source_IDE | Source_SCSI | Source_virtio_blk
> -(** Source disk controller.
> +(** Source disk controller (in ascending order of preference).
>  
>      For the purposes of this field, we can treat virtio-scsi as
>      [SCSI].  However we don't support conversions from virtio in any
> @@ -88,7 +88,7 @@ and source_nic = {
>    s_vnet_orig : string;                 (** Original network (if we map it). *)
>    s_vnet_type : vnet_type;              (** Source network type. *)
>  }
> -(** Network adapter models. *)
> +(** Network adapter models (in ascending order of preference). *)
>  and s_nic_model = Source_other_nic of string |
>                    Source_rtl8139 | Source_e1000 | Source_virtio_net
>  (** Network interfaces. *)
> @@ -108,7 +108,7 @@ and s_display_listen =
>    | LAddress of string             (** Listen address. *)
>    | LNetwork of string             (** Listen network. *)
>  
> -(** Video adapter model. *)
> +(** Video adapter model (in ascending order of preference). *)
>  and source_video = Source_other_video of string |
>                     Source_Cirrus | Source_QXL
>  
> diff --git a/v2v/v2v.ml b/v2v/v2v.ml
> index c828e48..608150f 100644
> --- a/v2v/v2v.ml
> +++ b/v2v/v2v.ml
> @@ -82,11 +82,17 @@ let rec main () =
>    );
>  
>    let keep_serial_console = output#keep_serial_console in
> -  let rcaps = {
> -                rcaps_block_bus = None;
> -                rcaps_net_bus = None;
> -                rcaps_video = None;
> -              } in
> +  let rcaps =
> +    match conversion_mode with
> +    | Copying (_, _) ->

You can just write this:

   | Copying _ ->

> +        {
> +          rcaps_block_bus = None;
> +          rcaps_net_bus = None;
> +          rcaps_video = None;
> +        }
> +    | In_place ->
> +        rcaps_from_source source
> +  in
>    let guestcaps = do_convert g inspect source keep_serial_console rcaps in
>  
>    g#umount_all ();
> @@ -974,4 +980,43 @@ and preserve_overlays overlays src_name =
>        printf (f_"Overlay saved as %s [--debug-overlays]\n") saved_filename
>    ) overlays
>  
> +and rcaps_from_source source =
> +  (* Request guest caps based on source configuration. *)
> +
> +  (* rely on s_controller enum being in ascending preference order, and None
> +   * being smaller than Some anything *)
> +  let best_block_type =
> +    List.fold_left max None
> +      (List.map (fun sd -> sd.s_controller) source.s_disks) in
> +  let block_type =
> +    match best_block_type with
> +    | Some Source_virtio_blk -> Some Virtio_blk
> +    | Some Source_SCSI -> None
> +    | Some Source_IDE -> Some IDE
> +    | None -> None in
> +
> +  (* rely on s_nic_model enum being in ascending preference order, and None
> +   * being smaller than Some anything *)
> +  let best_net_type =
> +    List.fold_left max None
> +      (List.map (fun nic -> nic.s_nic_model) source.s_nics) in
> +  let net_type =
> +    match best_net_type with
> +    | Some Source_virtio_net -> Some Virtio_net
> +    | Some Source_e1000 -> Some E1000
> +    | Some Source_rtl8139 -> Some RTL8139
> +    | Some Source_other_nic _ | None -> None in
> +
> +  let video =
> +    match source.s_video with
> +    | Some Source_QXL -> Some QXL
> +    | Some Source_Cirrus -> Some Cirrus
> +    | Some Source_other_video _ | None -> None in
> +
> +  {
> +    rcaps_block_bus = block_type;
> +    rcaps_net_bus = net_type;
> +    rcaps_video = video;
> +  }
> +
>  let () = run_main_and_handle_errors main

It's a bit surprising, because I thought that you'd want to pass in
the requested preferences on the command line?

About the specific issue of ordering, although what you've written
works, you could find a few surprises.  eg:

  $ rlwrap ocaml
          OCaml version 4.02.3
  
  # type t = A | B | C ;;
  type t = A | B | C
  # A < C ;;
  - : bool = true
  # A < B ;;
  - : bool = true
  # B < C ;;
  - : bool = true

but ...

  # type t = A | B of int | C ;;
  type t = A | B of int | C
  # A < C ;;
  - : bool = true
  # A < B 3 ;;
  - : bool = true
  # B 3 < C ;;
  - : bool = false

This happens because adding a parameter to an enum changes the
internal representation.

So the code as written could stop working with innocuous changes to
the declared types, or even in a future version of OCaml, because it
depends on internals of the OCaml representation.

IMHO it's safer to make the ordering you want explicit.  So:

  let precedence_of_s_controller_option = function
    | None -> 0 (* worst *)
    | Some Source_IDE -> 1
    | Some Source_SCSI -> 2
    | Some Source_virtio_blk -> 3 (* best *)

and then you can have a function that finds the best "something" in a
list of "somethings", using a polymorphic precedence function to
compare the best:

  let rec find_best precedence = function
    | [] -> None, 0
    | x :: xs ->
      let precedence_of_x = precedence x
      and best_in_xs, precedence_of_xs = find_best precedence xs in
      if precedence_of_x > precedence_of_xs then
        x, precedence_of_x
      else
        best_in_xs, precedence_of_xs

This function has type:

  val find_best : ('a option -> int) -> 'a option list -> 'a option * int

(Note it returns a pair: the best choice and also the precedence
integer of that choice).

The code to find the best block_type or net_type falls out fairly easily:

  let block_type =
    let sctrls = List.map (fun sd -> sd.s_controller) source.s_disks in
    let best, _ = find_best precedence_of_s_controller_option sctrls in
    match best with
    | Some Source_virtio_blk -> Some Virtio_blk
    | Some Source_SCSI -> None
    | Some Source_IDE -> Some IDE
    | None -> None in

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/