[Libguestfs] [PATCH 2/3] v2v: ovf: Create OVF more aligned with the standard

Richard W.M. Jones rjones at redhat.com
Tue Feb 20 10:08:34 UTC 2018


On Sun, Feb 18, 2018 at 03:26:09PM +0100, Tomáš Golembiovský wrote:
>  (* Create the OVF file. *)
>  let rec create_ovf source targets guestcaps inspect
> -    output_alloc sd_uuid image_uuids vol_uuids vm_uuid =
> +    output_alloc sd_uuid image_uuids vol_uuids vm_uuid rhv_export_flavor =

rhv_export_flavor is a boolean here.  TBH I'd use something
a bit more descriptive such as a variant:

  type ovf_flavour =
  | RHVExportStorageDomain
  | OVFStandard

and then code like this becomes:

> +      if rhv_export_flavor then
> +        e "Section" ["xsi:type", "ovf:NetworkSection_Type"] [
> +          e "Info" [] [PCData "List of networks"]
> +        ]
> +      else
> +        e "NetworkSection" [] [
> +          e "Info" [] [PCData "List of networks"]
> +        ];

  (match ovf_flavour with
  | RHVExportStorageDomain ->
     ...
  | OVFStandard ->
     ...
  )

Note the parentheses around match which are required.

This is implemented by OCaml in exactly the same way as a boolean so
there is no loss of efficiency.

>    let disk_section =
> -    let sections = path_to_nodes ovf ["ovf:Envelope"; "Section"] in
> -    try find_node_by_attr sections ("xsi:type", "ovf:DiskSection_Type")
> -    with Not_found -> assert false in
> +    let nodes = path_to_nodes ovf ["ovf:Envelope"; "DiskSection"] in
> +    match nodes with
> +    | [node] -> node
> +    | [] | _::_::_ -> let sections =
> +         path_to_nodes ovf ["ovf:Envelope"; "Section"] in
> +       try find_node_by_attr sections ("xsi:type", "ovf:DiskSection_Type")
> +       with Not_found -> assert false in

This pattern is repeated several times but I'm not really clear what
you're trying to do.  Isn't it an error if several nodes match the
first path?  In any case how about abstracting the pattern into a new
function:

  (* Add a comment explaining exactly what it does *)
  let get_section path1 path2 path2_attr =
    let nodes = path_to_nodes path1 in
    match nodes with
    | [node] -> node
    | _::_::_ -> assert false (* an error? *)
    | [] ->
      let nodes = path_to_nodes ovf path2 in
      try find_node_by_attr nodes path2_attr
      with Not_found -> assert false
  in

  (* ... *)

  let disk_section = get_section ["ovf:Envelope"; "Section"]
                                 ["ovf:Envelope"; "DiskSection"]
				 ("xsi:type", "ovf:DiskSection_Type") in

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW




More information about the Libguestfs mailing list