[Libguestfs] [PATCH 3/3] v2v: vdsm: add --vdsm-fixed-ovf option

[Richard W.M. Jones]

On Sun, Feb 18, 2018 at 03:26:10PM +0100, Tomáš Golembiovský wrote:
> Add option for -o vdsm that enables output of the modified OVF. oVirt
> engine should already be able to consume the OVF, but let's not take any
> chances and enable it only by command line argument. It can be made
> default later when it receives proper testing.
> 
> Signed-off-by: Tomáš Golembiovský <tgolembi at redhat.com>
> ---
>  v2v/cmdline.ml                           |  5 +++++
>  v2v/output_vdsm.ml                       |  8 ++++++--
>  v2v/output_vdsm.mli                      |  1 +
>  v2v/test-v2v-o-vdsm-options.ovf.expected | 26 +++++++++++++-------------
>  v2v/test-v2v-o-vdsm-options.sh           |  3 ++-
>  v2v/virt-v2v.pod                         |  7 +++++++
>  6 files changed, 34 insertions(+), 16 deletions(-)
> 
> diff --git a/v2v/cmdline.ml b/v2v/cmdline.ml
> index efad080cc..409d5dfab 100644
> --- a/v2v/cmdline.ml
> +++ b/v2v/cmdline.ml
> @@ -75,6 +75,7 @@ let parse_cmdline () =
>    let vddk_vimapiver = ref None in
>    let vdsm_vm_uuid = ref None in
>    let vdsm_ovf_output = ref None in (* default "." *)
> +  let vdsm_fixed_ovf = ref false in

The "fixed" OVF means standard OVF?  If so let's call it
vdsm_ovf_standard, which ties in with the suggestion in the previous
patch that we don't use a boolean but use a more descriptive type.

>    let vdsm_compat = ref "0.10" in
>    let set_vdsm_compat s = vdsm_compat := s in
> @@ -251,6 +252,8 @@ let parse_cmdline () =
>                                      s_"Output VM UUID";
>      [ L"vdsm-ovf-output" ], Getopt.String ("-", set_string_option_once "--vdsm-ovf-output" vdsm_ovf_output),
>                                      s_"Output OVF file";
> +    [ L"vdsm-fixed-ovf" ], Getopt.Set vdsm_fixed_ovf,
> +                                    s_"Produce OVF that is more conforming to the standard";
>      [ L"vmtype" ],   Getopt.String ("-", vmtype_warning),
>                                      s_"Ignored for backwards compatibility";
>    ] in
> @@ -327,6 +330,7 @@ read the man page virt-v2v(1).
>    let vdsm_vol_uuids = List.rev !vdsm_vol_uuids in
>    let vdsm_vm_uuid = !vdsm_vm_uuid in
>    let vdsm_ovf_output = Option.default "." !vdsm_ovf_output in
> +  let vdsm_fixed_ovf = !vdsm_fixed_ovf in
>  
>    (* No arguments and machine-readable mode?  Print out some facts
>     * about what this binary supports.
> @@ -542,6 +546,7 @@ read the man page virt-v2v(1).
>          vm_uuid = vdsm_vm_uuid;
>          ovf_output = vdsm_ovf_output;
>          compat = vdsm_compat;
> +        legacy_ovf = not vdsm_fixed_ovf;

Double negatives ...

The use of booleans here is very confusing, but generally the
patch is OK.

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