Libguestfs - Jan 2015 - Re: [PATCH] v2v: -o vdsm should assume data domain at -os path

On 20.01.15 14:48, Richard W.M. Jones wrote:
> On Tue, Jan 20, 2015 at 04:25:20PM +0200, Shahar Havivi wrote:
> > > > +    let pos = (String.length os) - 36 in
> > > > +    let mp = String.sub os 0 (pos-1) in
> > > > +    let uuid = String.sub os pos 36 in
> > > 
> > > What is the significance of 36 here?
> > the length of uuid.
> 
> That code above will throw an exception if you pass a too-short -os
> parameter.
> 
> How about this, which should be a little bit easier to understand:
> 
> let mp, uuid >   let fields = string_nsplit "/" os in (* ...
"data-center" "UUID" *)
>   let fields = List.rev fields in      (* "UUID"
"data-center" ... *)
>   match fields with
>   | "" :: uuid :: rest                 (* handles trailing
"/" case *)
>   | uuid :: rest
>       when String.length uuid = 36 ->
>     let mp = String.concat "/" (List.rev rest) in
>     mp, uuid
>   | _ ->
>     error (f_"vdsm: invalid -os parameter does not contain a valid
UUID: %s")
>       os
> 
Looks better, I will give it a try
> Or perhaps something with regexps ...
> 
> Rich.
> 
> -- 
> Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
> Read my programming and virtualization blog: http://rwmj.wordpress.com
> libguestfs lets you edit virtual machines.  Supports shell scripting,
> bindings from many languages.  http://libguestfs.org

On Tue, Jan 20, 2015 at 05:19:54PM +0200, Shahar Havivi
wrote:
> >From ff9261b201283f7bff16dc41721480e2e0a9f80f Mon Sep 17 00:00:00 2001
> From: Shahar Havivi <shaharh@redhat.com>
> Date: Tue, 20 Jan 2015 14:41:23 +0200
> Subject: [PATCH] v2v: -o vdsm should assume data domain at -os path
> 
> Unlike -o rhev which have only one data domin, -o vdsm can and usually
> does have multiple data domain.
> The path to vdsm is pre mounted so no need to assume nfs path with -os
> Example:
> -o vdsm -os
/rhev/data-center/<data-center-uuid>/<data-domain-uuid>
> 
> Bug-Url: https://bugzilla.redhat.com/1176591
> Signed-off-by: Shahar Havivi <shaharh@redhat.com>
> ---
>  v2v/output_vdsm.ml | 18 ++++++++++++++----
>  v2v/virt-v2v.pod   |  6 ++++--
>  2 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/v2v/output_vdsm.ml b/v2v/output_vdsm.ml
> index 492f586..c7a243e 100644
> --- a/v2v/output_vdsm.ml
> +++ b/v2v/output_vdsm.ml
> @@ -63,9 +63,8 @@ object
>     * name of the target files that eventually get written by the main
>     * code.
>     *
> -   * 'os' is the output storage (-os nfs:/export). 
'source' contains a
> -   * few useful fields such as the guest name.  'targets'
describes the
> -   * destination files.  We modify and return this list.
> +   * 'os' is the output storage domain (-os /rhev/data/<data
center>/<data domain>)
> +   * this is already mounted path.
>     *
>     * Note it's good to fail here (early) if there are any problems,
since
>     * the next time we are called (in {!create_metadata}) we have already
> @@ -79,7 +78,18 @@ object
>          (List.length targets);
>  
>      let mp, uuid > -      Output_rhev.mount_and_check_storage_domain
verbose (s_"Data Domain") os in
> +      let fields = string_nsplit "/" os in (* ...
"data-center" "UUID" *)
> +      let fields = List.rev fields in      (* "UUID"
"data-center" ... *)
> +      match fields with
> +      | "" :: uuid :: rest                 (* handles trailing
"/" case *)
> +      | uuid :: rest
> +          when String.length uuid = 36 ->
> +        let mp = String.concat "/" (List.rev rest) in
> +        mp, uuid
> +      | _ ->
> +        error (f_"vdsm: invalid -os parameter does not contain a
valid UUID: %s")
> +          os in
> +
>      dd_mp <- mp;
>      dd_uuid <- uuid;
>      if verbose then
> diff --git a/v2v/virt-v2v.pod b/v2v/virt-v2v.pod
> index a48bf59..2eb0079 100644
> --- a/v2v/virt-v2v.pod
> +++ b/v2v/virt-v2v.pod
> @@ -456,8 +456,10 @@ See L</OUTPUT TO RHEV> below.
>  
>  Set the output method to I<vdsm>.
>  
> -This mode is similar to I<-o rhev> but is only used by RHEV VDSM
> -when it runs virt-v2v under VDSM control.
> +This mode is similar to I<-o rhev>, but the full path to the
> +data domain must be given:
>
+C</rhev/data-center/E<lt>data-center-uuidE<gt>/E<lt>data-domain-uuidE<gt>>.
> +This mode is only used when virt-v2v runs under VDSM control.
>  
>  =item B<-oa sparse>
Sorry for forgetting about this patch.  The tests don't pass when this
patch is applied.  I'm not sure if the tests need to be modified or if
the patch itself contains a mistake.

Also the patch itself doesn't apply without hand-editing.  Please
install 'git-email' and use the 'git send-email' command to send
patches to the list.

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