Libguestfs - Oct 2017 - [PATCH 0/5] v2v: Handle disks with snapshots (RHBZ#1172425).

The first commit removes the --dcpath parameter, which just makes the
following stuff simpler.  Since libvirt has supported getting
datacenterpath from VMware since Oct 2015, it's time to drop this
hairy parameter.

The rest is quite a complicated series of refactorings, followed by a
very simple change to add handling of snapshots taken from old
virt-v2v.

Rich.

With modern libvirt, when fetching the XML of a VMware guest libvirt
passes us the datacenter path (dcpath).  However with older libvirt we
had to guess this value, or else the user had to supply it on the
command line.

This commit removes all the guessing code and the --dcpath parameter
(which will now give an error).

This requires libvirt >= 1.2.20 for virt-v2v, released Oct 2015.
---
 v2v/cmdline.ml                      |  6 +----
 v2v/copy_to_local.ml                |  6 +++--
 v2v/input_libvirt.ml                |  4 ++--
 v2v/input_libvirt.mli               |  4 ++--
 v2v/input_libvirt_vcenter_https.ml  | 31 +++++++-----------------
 v2v/input_libvirt_vcenter_https.mli |  2 +-
 v2v/test-v2v-docs.sh                |  2 +-
 v2v/vCenter.ml                      | 47 +------------------------------------
 v2v/vCenter.mli                     |  8 -------
 v2v/virt-v2v.pod                    | 13 ----------
 10 files changed, 20 insertions(+), 103 deletions(-)

diff --git a/v2v/cmdline.ml b/v2v/cmdline.ml
index 1ae018bcd..a5f4f7846 100644
--- a/v2v/cmdline.ml
+++ b/v2v/cmdline.ml
@@ -55,7 +55,6 @@ let parse_cmdline ()    let print_source = ref false in
   let qemu_boot = ref false in
 
-  let dcpath = ref None in
   let input_conn = ref None in
   let input_format = ref None in
   let in_place = ref false in
@@ -182,8 +181,6 @@ let parse_cmdline ()    let argspec = [
     [ S 'b'; L"bridge" ],        Getopt.String
("in:out", add_bridge),     s_"Map bridge ‘in’ to ‘out’";
     [ L"compressed" ], Getopt.Set compressed,     s_"Compress
output file (-of qcow2 only)";
-    [ L"dcpath"; L"dcPath" ],  Getopt.String
("path", set_string_option_once "--dcpath" dcpath),
-                                            s_"Override dcPath (for
vCenter)";
     [ L"debug-overlay"; L"debug-overlays" ], Getopt.Set
debug_overlays, s_"Save overlay files";
     [ S 'i' ],        Getopt.String (i_options, set_input_mode),
s_"Set input mode (default: libvirt)";
     [ M"ic" ],       Getopt.String ("uri",
set_string_option_once "-ic" input_conn),
@@ -270,7 +267,6 @@ read the man page virt-v2v(1).
   (* Dereference the arguments. *)
   let args = List.rev !args in
   let compressed = !compressed in
-  let dcpath = !dcpath in
   let debug_overlays = !debug_overlays in
   let do_copy = !do_copy in
   let input_conn = !input_conn in
@@ -368,7 +364,7 @@ read the man page virt-v2v(1).
         | [guest] -> guest
         | _ ->
           error (f_"expecting a libvirt guest name on the command
line") in
-      Input_libvirt.input_libvirt dcpath vddk_options password input_conn guest
+      Input_libvirt.input_libvirt vddk_options password input_conn guest
 
     | `LibvirtXML ->
       (* -i libvirtxml: Expecting a filename (XML file). *)
diff --git a/v2v/copy_to_local.ml b/v2v/copy_to_local.ml
index 8a64f3a58..63e1ea310 100644
--- a/v2v/copy_to_local.ml
+++ b/v2v/copy_to_local.ml
@@ -245,8 +245,10 @@ and parse_libvirt_xml guest_name xml                       
"vmware" "http://libvirt.org/schemas/domain/vmware/1.0";
   let xpath_string = xpath_string xpathctx in
 
-  (* Get the dcpath, only present for libvirt >= 1.2.20 so use a
-   * sensible default for older versions.
+  (* Get the dcpath, present in libvirt >= 1.2.20.
+   * XXX Unfortunately when testing virt-v2v-copy-to-local against a
+   * local libvirt test XML file, we cannot pass in the vmware namespace.
+   * So we must keep the default here until we can resolve that somehow.
    *)
   let dcpath      Option.default "ha-datacenter"
diff --git a/v2v/input_libvirt.ml b/v2v/input_libvirt.ml
index 708feccc7..d05116ac6 100644
--- a/v2v/input_libvirt.ml
+++ b/v2v/input_libvirt.ml
@@ -27,7 +27,7 @@ open Types
 open Utils
 
 (* Choose the right subclass based on the URI. *)
-let input_libvirt dcpath vddk_options password libvirt_uri guest +let
input_libvirt vddk_options password libvirt_uri guest    match libvirt_uri with
   | None ->
     Input_libvirt_other.input_libvirt_other password libvirt_uri guest
@@ -54,7 +54,7 @@ let input_libvirt dcpath vddk_options password libvirt_uri
guest         (match vddk_options with
         | None ->
            Input_libvirt_vcenter_https.input_libvirt_vcenter_https
-             dcpath password libvirt_uri parsed_uri scheme server guest
+             password libvirt_uri parsed_uri scheme server guest
         | Some vddk_options ->
            Input_libvirt_vddk.input_libvirt_vddk vddk_options password
                                                  libvirt_uri parsed_uri guest
diff --git a/v2v/input_libvirt.mli b/v2v/input_libvirt.mli
index 0a6aa3c54..acf2ca417 100644
--- a/v2v/input_libvirt.mli
+++ b/v2v/input_libvirt.mli
@@ -18,7 +18,7 @@
 
 (** [-i libvirt] source. *)
 
-val input_libvirt : string option -> Types.vddk_options option -> string
option -> string option -> string -> Types.input
-(** [input_libvirt dcpath vddk_options password libvirt_uri guest] creates
+val input_libvirt : Types.vddk_options option -> string option -> string
option -> string -> Types.input
+(** [input_libvirt vddk_options password libvirt_uri guest] creates
     and returns a new {!Types.input} object specialized for reading input
     from libvirt sources. *)
diff --git a/v2v/input_libvirt_vcenter_https.ml
b/v2v/input_libvirt_vcenter_https.ml
index fef64583f..4c2fb3cc1 100644
--- a/v2v/input_libvirt_vcenter_https.ml
+++ b/v2v/input_libvirt_vcenter_https.ml
@@ -36,7 +36,7 @@ let readahead_for_copying = Some (64 * 1024 * 1024)
 
 (* Subclass specialized for handling VMware vCenter over https. *)
 class input_libvirt_vcenter_https
-  cmdline_dcPath password libvirt_uri parsed_uri scheme server guest +       
password libvirt_uri parsed_uri scheme server guest  object
   inherit input_libvirt password libvirt_uri guest
 
@@ -67,33 +67,18 @@ object
     let xml = Libvirt_utils.dumpxml ?password ?conn:libvirt_uri guest in
     let source, disks = parse_libvirt_xml ?conn:libvirt_uri xml in
 
-    (* Find the <vmware:datacenterpath> element from the XML, if it
-     * exists.  This was added in libvirt >= 1.2.20.
+    (* Find the <vmware:datacenterpath> element from the XML.  This
+     * was added in libvirt >= 1.2.20.
      *)
-    let xml_dcPath +    dcPath <- (
       let doc = Xml.parse_memory xml in
       let xpathctx = Xml.xpath_new_context doc in
       Xml.xpath_register_ns xpathctx
         "vmware"
"http://libvirt.org/schemas/domain/vmware/1.0";
-      let xpath_string = xpath_string xpathctx in
-      xpath_string "/domain/vmware:datacenterpath" in
-
-    (* Calculate the dcPath we're going to use. *)
-    dcPath <- (
-      match cmdline_dcPath, xml_dcPath with
-      (* Command line --dcpath parameter overrides everything, allowing
-       * users to correct any mistakes in v2v or libvirt.
-       *)
-      | Some p, (None|Some _) ->
-         debug "vcenter: using --dcpath from the command line: %s" p;
-         p
-      | None, Some p ->
-         debug "vcenter: using <vmware:datacenterpath> from libvirt:
%s" p;
-         p
-      | None, None ->
-         let p = VCenter.guess_dcPath parsed_uri scheme in
-         debug "vcenter: guessed dcPath from URI: %s" p;
-         p
+      match xpath_string xpathctx "/domain/vmware:datacenterpath"
with
+      | Some dcPath -> dcPath
+      | None ->
+         error (f_"vcenter: <vmware:datacenterpath> was not found in
the XML.  You need to upgrade to libvirt ≥ 1.2.20.")
     );
 
     (* Save the original source paths, so that we can remap them again
diff --git a/v2v/input_libvirt_vcenter_https.mli
b/v2v/input_libvirt_vcenter_https.mli
index 840b5a90f..d347f5fe6 100644
--- a/v2v/input_libvirt_vcenter_https.mli
+++ b/v2v/input_libvirt_vcenter_https.mli
@@ -18,4 +18,4 @@
 
 (** [-i libvirt] when the source is VMware vCenter *)
 
-val input_libvirt_vcenter_https : string option -> string option ->
string option -> Xml.uri -> string -> string -> string ->
Types.input
+val input_libvirt_vcenter_https : string option -> string option ->
Xml.uri -> string -> string -> string -> Types.input
diff --git a/v2v/test-v2v-docs.sh b/v2v/test-v2v-docs.sh
index d341852b7..8bd03f68e 100755
--- a/v2v/test-v2v-docs.sh
+++ b/v2v/test-v2v-docs.sh
@@ -22,4 +22,4 @@ $TEST_FUNCTIONS
 skip_if_skipped
 
 $top_srcdir/podcheck.pl virt-v2v.pod virt-v2v \
- 
--ignore=--dcPath,--debug-overlay,--ic,--if,--no-trim,--oa,--oc,--of,--on,--os,--vmtype
+ 
--ignore=--debug-overlay,--ic,--if,--no-trim,--oa,--oc,--of,--on,--os,--vmtype
diff --git a/v2v/vCenter.ml b/v2v/vCenter.ml
index 41bdbdd01..e244ff4d5 100644
--- a/v2v/vCenter.ml
+++ b/v2v/vCenter.ml
@@ -86,7 +86,7 @@ let get_session_cookie password scheme uri sslverify url  
     if status = "404" then (
       dump_response stderr;
-      error (f_"vcenter: URL not found: %s\n\nThe ‘--dcpath’ parameter may
be useful.  See the explanation in the virt-v2v(1) man page OPTIONS
section.") url
+      error (f_"vcenter: URL not found: %s") url
     );
 
     if status <> "200" then (
@@ -113,51 +113,6 @@ let get_session_cookie password scheme uri sslverify url   
Some !session_cookie
   )
 
-let multiple_slash = PCRE.compile "/{2,}"
-let default_dc = "ha-datacenter"
-
-let guess_dcPath uri = function
-  | "vpx" ->
-     (match uri.uri_path with
-      | None ->
-         warning (f_"vcenter: URI (-ic parameter) contains no path, so we
cannot determine the dcPath (datacenter name)");
-         default_dc
-      | Some path ->
-         (* vCenter: URIs are *usually* '/Folder/Datacenter/esxi' so we
can
-          * just chop off the first '/' and final '/esxi' to
get the dcPath.
-          *
-          * The libvirt driver allows things like '/DC///esxi////' so
we also
-          * have to handle trailing slashes and collapse multiple slashes into
-          * single (RHBZ#1258342).
-          *
-          * However if there is a cluster involved then the URI may be
-          * /Folder/Datacenter/Cluster/esxi but
dcPath=Folder/Datacenter/Cluster
-          * won't work.  In this case the user has to adjust the path to
-          * remove the Cluster name (which still works in libvirt).
-          *)
-         (* Collapse multiple slashes to single slash. *)
-         let path = PCRE.replace ~global:true multiple_slash "/" path
in
-         (* Chop off the first and trailing '/' (if found). *)
-         let path -           let len = String.length path in
-           if len > 0 && path.[0] = '/' then
-             String.sub path 1 (len-1)
-           else path in
-         let path -           let len = String.length path in
-           if len > 0 && path.[len-1] = '/' then
-             String.sub path 0 (len-1)
-           else path in
-         (* Chop off the final element (ESXi hostname). *)
-         let len -           try String.rindex path '/' with Not_found
-> String.length path in
-         String.sub path 0 len
-     );
-  | "esx" -> (* Connecting to an ESXi hypervisor directly, so
it's fixed. *)
-     default_dc
-  | _ ->     (* Don't know, so guess. *)
-     default_dc
-
 let source_re = PCRE.compile "^\\[(.*)\\] (.*)\\.vmdk$"
 
 let map_source_to_https dcPath uri server path diff --git a/v2v/vCenter.mli
b/v2v/vCenter.mli
index 224f45009..55d70b486 100644
--- a/v2v/vCenter.mli
+++ b/v2v/vCenter.mli
@@ -35,14 +35,6 @@ val get_session_cookie : string option -> string ->
Xml.uri -> bool -> string ->
     The session cookie is memoized so you can call this function as
     often as you want, and only a single log in is made. *)
 
-val guess_dcPath : Xml.uri -> string -> string
-(** Try to guess the dcPath parameter from a URI.  The mapping is
-    not precise.
-
-    This function is only used with [libvirt < 1.2.20] because later
-    versions of libvirt provide the dcPath (see
-    https://bugzilla.redhat.com/1263574). *)
-
 val map_source_to_uri : int option -> string -> string option ->
Xml.uri -> string -> string -> string -> string
 (** [map_source_to_uri readahead dcPath password uri scheme server path]
     maps the [<source path=...>] string to a qemu URI.
diff --git a/v2v/virt-v2v.pod b/v2v/virt-v2v.pod
index 28292f6e3..199a7dd8b 100644
--- a/v2v/virt-v2v.pod
+++ b/v2v/virt-v2v.pod
@@ -282,19 +282,6 @@ Write a compressed output file.  This is only allowed if
the output
 format is qcow2 (see I<-of> below), and is equivalent to the I<-c>
 option of L<qemu-img(1)>.
 
-=item B<--dcpath> Folder/Datacenter
-
-B<NB:> You don’t need to use this parameter if you have
-S<libvirt E<ge> 1.2.20>.
-
-For VMware vCenter, override the C<dcPath=...> parameter used to
-select the datacenter.  Virt-v2v can usually calculate this from the
-C<vpx://> URI, but if it gets it wrong, then you can override it using
-this setting.  Go to your vCenter web folder interface, eg.
-C<https://vcenter.example.com/folder> (I<without> a trailing
slash),
-and examine the C<dcPath=> parameter in the URLs that appear on this
-page.
-
 =item B<--debug-overlays>
 
 Save the overlay file(s) created during conversion.  This option is
-- 
2.13.2

This module had a selection of functions taking a different mix of
parameters and doing slightly different things.  You could call one
function to return an https://... URL, or another function to return a
qemu URL, and there was a third function to get the session cookie but
you generally had to call that anyway (and it was implicitly called
when making the qemu URL!)

Integrate these into a single function which returns a struct
returning all possible values.

This is conceptually refactoring, except that the session cookie is no
longer memoized, but we didn't use this feature (of calling
get_session_cookie multiple times) anyway.
---
 v2v/copy_to_local.ml               |  12 +-
 v2v/input_libvirt_vcenter_https.ml |  12 +-
 v2v/vCenter.ml                     | 317 +++++++++++++++++++------------------
 v2v/vCenter.mli                    |  66 +++++---
 4 files changed, 210 insertions(+), 197 deletions(-)

diff --git a/v2v/copy_to_local.ml b/v2v/copy_to_local.ml
index 63e1ea310..5773dcff1 100644
--- a/v2v/copy_to_local.ml
+++ b/v2v/copy_to_local.ml
@@ -147,14 +147,10 @@ read the man page virt-v2v-copy-to-local(1).
     | ESXi server ->
        List.map (
          fun (remote_disk, local_disk) ->
-           let url, sslverify -             VCenter.map_source_to_https dcpath
parsed_uri
-                                         server remote_disk in
-           debug "esxi: source disk %s (sslverify=%b)" url sslverify;
-           let cookie -             VCenter.get_session_cookie password
"esx"
-                                        parsed_uri sslverify url in
-           (url, local_disk, sslverify, cookie)
+           let { VCenter.https_url; sslverify; session_cookie } +            
VCenter.map_source dcpath parsed_uri "esx" server remote_disk in
+           debug "esxi: source disk %s (sslverify=%b)" https_url
sslverify;
+           (https_url, local_disk, sslverify, session_cookie)
        ) disks
     | Test | Xen_ssh _ ->
        List.map (fun (remote_disk, local_disk) ->
diff --git a/v2v/input_libvirt_vcenter_https.ml
b/v2v/input_libvirt_vcenter_https.ml
index 4c2fb3cc1..79b55cd5a 100644
--- a/v2v/input_libvirt_vcenter_https.ml
+++ b/v2v/input_libvirt_vcenter_https.ml
@@ -101,9 +101,9 @@ object
       | { p_source = P_source_dev _ } -> assert false
       | { p_source_disk = disk; p_source = P_dont_rewrite } -> disk
       | { p_source_disk = disk; p_source = P_source_file path } ->
-        let qemu_uri -          VCenter.map_source_to_uri readahead dcPath
password
-                                    parsed_uri scheme server path in
+        let { VCenter.qemu_uri } +          VCenter.map_source ?readahead
?password
+                             dcPath parsed_uri scheme server path in
 
         (* The libvirt ESX driver doesn't normally specify a format, but
          * the format of the -flat file is *always* raw, so force it here.
@@ -122,9 +122,9 @@ object
     | None -> ()
     | Some orig_path ->
       let readahead = readahead_for_copying in
-      let backing_qemu_uri -        VCenter.map_source_to_uri readahead dcPath
password
-                                  parsed_uri scheme server orig_path in
+      let { VCenter.qemu_uri = backing_qemu_uri } +        VCenter.map_source
?readahead ?password
+                           dcPath parsed_uri scheme server orig_path in
 
       (* Rebase the qcow2 overlay to adjust the readahead parameter. *)
       let cmd = [ Guestfs_config.qemu_img; "rebase"; "-u";
diff --git a/v2v/vCenter.ml b/v2v/vCenter.ml
index e244ff4d5..2962618d4 100644
--- a/v2v/vCenter.ml
+++ b/v2v/vCenter.ml
@@ -25,166 +25,167 @@ open Common_gettext.Gettext
 open Xml
 open Utils
 
-(* Memoized session cookie. *)
-let session_cookie = ref ""
-
-let get_session_cookie password scheme uri sslverify url -  if !session_cookie
<> "" then
-    Some !session_cookie
-  else (
-    let curl_args = ref [
-      "head", None;
-      "silent", None;
-      "url", Some url;
-    ] in
-    (match uri.uri_user, password with
-     | None, None -> ()
-     | None, Some _ ->
-        warning (f_"--password-file parameter ignored because
'user@' was not given in the URL")
-     | Some user, None ->
-        List.push_back curl_args ("user", Some user)
-     | Some user, Some password ->
-        List.push_back curl_args ("user", Some (user ^ ":"
^ password))
-    );
-    if not sslverify then List.push_back curl_args ("insecure",
None);
-
-    let curl_h = Curl.create !curl_args in
-    let lines = Curl.run curl_h in
-
-    let dump_response chan -      Curl.print chan curl_h;
-
-      (* Dump out the output of the command. *)
-      List.iter (fun x -> fprintf chan "%s\n" x) lines;
-      flush chan
-    in
-
-    if verbose () then dump_response stderr;
-
-    (* Look for the last HTTP/x.y NNN status code in the output. *)
-    let status = ref "" in
-    List.iter (
-      fun line ->
-        let len = String.length line in
-        if len >= 12 && String.sub line 0 5 = "HTTP/" then
-          status := String.sub line 9 3
-    ) lines;
-    let status = !status in
-    if status = "" then (
-      dump_response stderr;
-      error (f_"vcenter: no status code in output of ‘curl’ command.  Is
‘curl’ installed?")
-    );
-
-    if status = "401" then (
-      dump_response stderr;
-      if uri.uri_user <> None then
-        error (f_"vcenter: incorrect username or password")
-      else
-        error (f_"vcenter: incorrect username or password.  You might need
to specify the username in the URI like this: %s://USERNAME@[etc]")
-              scheme
-    );
-
-    if status = "404" then (
-      dump_response stderr;
-      error (f_"vcenter: URL not found: %s") url
-    );
-
-    if status <> "200" then (
-      dump_response stderr;
-      error (f_"vcenter: invalid response from server")
-    );
-
-    (* Get the cookie. *)
-    List.iter (
-      fun line ->
-        let len = String.length line in
-        if len >= 12 && String.sub line 0 12 = "Set-Cookie:
" then (
-          let line = String.sub line 12 (len-12) in
-          let cookie, _ = String.split ";" line in
-          session_cookie := cookie
-        )
-    ) lines;
-    if !session_cookie = "" then (
-      dump_response stderr;
-      warning (f_"vcenter: could not read session cookie from the vCenter
Server, conversion may consume all sessions on the server and fail part way
through");
-      None
-    )
-    else
-      Some !session_cookie
-  )
+type remote_resource = {
+  https_url : string;
+  qemu_uri : string;
+  session_cookie : string option;
+  sslverify : bool;
+}
 
 let source_re = PCRE.compile "^\\[(.*)\\] (.*)\\.vmdk$"
 
-let map_source_to_https dcPath uri server path -  if not (PCRE.matches
source_re path) then
-    (path, true)
-  else (
-    let datastore = PCRE.sub 1 and path = PCRE.sub 2 in
-
-    let port -      match uri.uri_port with
-      | 443 -> ""
-      | n when n >= 1 -> ":" ^ string_of_int n
-      | _ -> "" in
-
-    (* XXX Old virt-v2v could also handle snapshots, ie:
-     * "[datastore1] Fedora 20/Fedora 20-NNNNNN.vmdk"
-     * XXX Need to handle templates.  The file is called
"-delta.vmdk" in
-     * place of "-flat.vmdk".
-     *)
-    let url -      sprintf
-        "https://%s%s/folder/%s-flat.vmdk?dcPath=%s&dsName=%s"
-        server port
-        (uri_quote path) (uri_quote dcPath) (uri_quote datastore) in
-
-    (* If no_verify=1 was passed in the libvirt URI, then we have to
-     * turn off certificate verification here too.
-     *)
-    let sslverify -      match uri.uri_query_raw with
-      | None -> true
-      | Some query ->
-        (* XXX only works if the query string is not URI-quoted *)
-        String.find query "no_verify=1" = -1 in
-
-    (url, sslverify)
-  )
-
-let map_source_to_uri readahead dcPath password uri scheme server path -  let
url, sslverify = map_source_to_https dcPath uri server path in
-
-  (* Now we have to query the server to get the session cookie. *)
-  let session_cookie = get_session_cookie password scheme uri sslverify url in
-
-  (* Construct the JSON parameters. *)
-  let json_params = [
-    "file.driver", JSON.String "https";
-    "file.url", JSON.String url;
-    (* https://bugzilla.redhat.com/show_bug.cgi?id=1146007#c10 *)
-    "file.timeout", JSON.Int 2000;
-  ] in
-
-  let json_params -    match readahead with
-    | None -> json_params
-    | Some readahead ->
-       ("file.readahead", JSON.Int readahead) :: json_params in
-
-  let json_params -    if sslverify then json_params
-    else ("file.sslverify", JSON.String "off") ::
json_params in
-
-  let json_params -    match session_cookie with
-    | None -> json_params
-    | Some cookie -> ("file.cookie", JSON.String cookie) ::
json_params in
-
-  debug "vcenter: json parameters: %s" (JSON.string_of_doc
json_params);
-
-  (* Turn the JSON parameters into a 'json:' protocol string.
-   * Note this requires qemu-img >= 2.1.0.
+let rec map_source ?readahead ?password dcPath uri scheme server path +  (* If
no_verify=1 was passed in the libvirt URI, then we have to
+   * turn off certificate verification here too.
    *)
-  let qemu_uri = "json: " ^ JSON.string_of_doc json_params in
+  let sslverify +    match uri.uri_query_raw with
+    | None -> true
+    | Some query ->
+       (* XXX only works if the query string is not URI-quoted *)
+       String.find query "no_verify=1" = -1 in
 
-  qemu_uri
+  let https_url +    if not (PCRE.matches source_re path) then
+      path
+    else (
+      let datastore = PCRE.sub 1 and path = PCRE.sub 2 in
+
+      let port +        match uri.uri_port with
+        | 443 -> ""
+        | n when n >= 1 -> ":" ^ string_of_int n
+        | _ -> "" in
+
+      (* XXX Old virt-v2v could also handle snapshots, ie:
+       * "[datastore1] Fedora 20/Fedora 20-NNNNNN.vmdk"
+       * XXX Need to handle templates.  The file is called
"-delta.vmdk" in
+       * place of "-flat.vmdk".
+       *)
+      sprintf
"https://%s%s/folder/%s-flat.vmdk?dcPath=%s&dsName=%s"
+              server port
+              (uri_quote path) (uri_quote dcPath) (uri_quote datastore)
+    ) in
+
+  let session_cookie +    get_session_cookie password scheme uri sslverify
https_url in
+
+  let qemu_uri +    (* Construct the JSON parameters for the qemu URI. *)
+    let json_params = [
+      "file.driver", JSON.String "https";
+      "file.url", JSON.String https_url;
+      (* https://bugzilla.redhat.com/show_bug.cgi?id=1146007#c10 *)
+      "file.timeout", JSON.Int 2000;
+    ] in
+
+    let json_params +      match readahead with
+      | None -> json_params
+      | Some readahead ->
+         ("file.readahead", JSON.Int readahead) :: json_params in
+
+    let json_params +      if sslverify then json_params
+      else ("file.sslverify", JSON.String "off") ::
json_params in
+
+    let json_params +      match session_cookie with
+      | None -> json_params
+      | Some cookie -> ("file.cookie", JSON.String cookie) ::
json_params in
+
+    debug "vcenter: json parameters: %s" (JSON.string_of_doc
json_params);
+
+    (* Turn the JSON parameters into a 'json:' protocol string.
+     * Note this requires qemu-img >= 2.1.0.
+     *)
+    let qemu_uri = "json: " ^ JSON.string_of_doc json_params in
+
+    qemu_uri in
+
+  (* Return the struct. *)
+  { https_url = https_url;
+    qemu_uri = qemu_uri;
+    session_cookie = session_cookie;
+    sslverify = sslverify }
+
+and get_session_cookie password scheme uri sslverify https_url +  let curl_args
= ref [
+    "head", None;
+    "silent", None;
+    "url", Some https_url;
+  ] in
+  (match uri.uri_user, password with
+   | None, None -> ()
+   | None, Some _ ->
+      warning (f_"--password-file parameter ignored because
'user@' was not given in the URL")
+   | Some user, None ->
+      List.push_back curl_args ("user", Some user)
+   | Some user, Some password ->
+      List.push_back curl_args ("user", Some (user ^ ":" ^
password))
+  );
+  if not sslverify then List.push_back curl_args ("insecure", None);
+
+  let curl_h = Curl.create !curl_args in
+  let lines = Curl.run curl_h in
+
+  let dump_response chan +    Curl.print chan curl_h;
+
+    (* Dump out the output of the command. *)
+    List.iter (fun x -> fprintf chan "%s\n" x) lines;
+    flush chan
+  in
+
+  if verbose () then dump_response stderr;
+
+  (* Look for the last HTTP/x.y NNN status code in the output. *)
+  let status = ref "" in
+  List.iter (
+    fun line ->
+      let len = String.length line in
+      if len >= 12 && String.sub line 0 5 = "HTTP/" then
+        status := String.sub line 9 3
+  ) lines;
+  let status = !status in
+  if status = "" then (
+    dump_response stderr;
+    error (f_"vcenter: no status code in output of ‘curl’ command.  Is
‘curl’ installed?")
+  );
+
+  if status = "401" then (
+    dump_response stderr;
+    if uri.uri_user <> None then
+      error (f_"vcenter: incorrect username or password")
+    else
+      error (f_"vcenter: incorrect username or password.  You might need
to specify the username in the URI like this: %s://USERNAME@[etc]")
+            scheme
+  );
+
+  if status = "404" then (
+    dump_response stderr;
+    error (f_"vcenter: URL not found: %s") https_url
+  );
+
+  if status <> "200" then (
+    dump_response stderr;
+    error (f_"vcenter: invalid response from server")
+  );
+
+  (* Get the cookie. *)
+  let rec loop = function
+    | [] ->
+       dump_response stderr;
+       warning (f_"vcenter: could not read session cookie from the vCenter
Server, conversion may consume all sessions on the server and fail part way
through");
+       None
+    | line :: lines ->
+       let len = String.length line in
+       if len >= 12 && String.sub line 0 12 = "Set-Cookie:
" then (
+         let line = String.sub line 12 (len-12) in
+         let cookie, _ = String.split ";" line in
+         Some cookie
+       )
+       else
+         loop lines
+  in
+  loop lines
diff --git a/v2v/vCenter.mli b/v2v/vCenter.mli
index 55d70b486..03749966f 100644
--- a/v2v/vCenter.mli
+++ b/v2v/vCenter.mli
@@ -18,35 +18,51 @@
 
 (** Functions for dealing with VMware vCenter. *)
 
-val get_session_cookie : string option -> string -> Xml.uri -> bool
-> string -> string option
-(** [get_session_cookie password scheme uri sslverify url]
-    contacts the vCenter server, logs in, and gets the session cookie,
-    which can later be passed back to the server instead of having to
-    log in each time (this is also more efficient since it avoids
-    vCenter running out of authentication sessions).
+type remote_resource = {
+  https_url : string;
+  (** The full URL of the remote disk as an https link on the vCenter
+      server.  It will have the general form
+      [https://vcenter/folder/.../guest-flat.vmdk?dcPath=...&...] *)
 
-    Returns [None] if the session cookie could not be read (but
-    authentication was successful).  You can proceed without the
-    session cookie in this case, but there is an unavoidable
-    danger of running out of authentication sessions.  If the
-    session cookie could not be read, this function prints a
-    warning.
+  qemu_uri : string;
+  (** The remote disk as a QEMU URI.  This opaque blob (usually a
+      [json:] URL) can be passed to [qemu] or [qemu-img] as a backing
+      file. *)
 
-    The session cookie is memoized so you can call this function as
-    often as you want, and only a single log in is made. *)
+  session_cookie : string option;
+  (** When creating the URLs above, the module contacts the vCenter
+      server, logs in, and gets the session cookie, which can later
+      be passed back to the server instead of having to log in each
+      time (this is also more efficient since it avoids vCenter
+      running out of authentication sessions).
 
-val map_source_to_uri : int option -> string -> string option ->
Xml.uri -> string -> string -> string -> string
-(** [map_source_to_uri readahead dcPath password uri scheme server path]
-    maps the [<source path=...>] string to a qemu URI.
+      This can be [None] if the session cookie could not be read (but
+      authentication was successful).  You can proceed without the
+      session cookie in this case, but there is an unavoidable
+      danger of running out of authentication sessions.  If the
+      session cookie could not be read, this function prints a
+      warning.
 
-    The [path] will be something like:
+      If authentication {i failed} then the {!map_source} function
+      would exit with an error, so [None] does not indicate auth
+      failure. *)
 
+  sslverify : bool;
+  (** This is true except when the libvirt URI had [?no_verify=1] in
+      the parameters. *)
+}
+(** The "remote resource" is the structure returned by the
{!map_source}
+    function. *)
+
+val map_source : ?readahead:int -> ?password:string -> string ->
Xml.uri -> string -> string -> string -> remote_resource
+(** [map_source ?readahead ?password dcPath uri scheme server path]
+    maps the [<source path=...>] string to a {!remote_resource}
+    structure containing both an [https://] URL and a qemu URI,
+    both pointing the guest disk.
+
+    The input [path] comes from libvirt and will be something like:
     ["[datastore1] Fedora 20/Fedora 20.vmdk"]
+    (including those literal spaces in the string).
 
-    including those literal spaces in the string. *)
-
-val map_source_to_https : string -> Xml.uri -> string -> string ->
string * bool
-(** [map_source_to_https dcPath uri server path] is the same as
-    {!map_source_to_uri} but it produces a regular [https://...] URL.
-    The returned boolean is whether TLS certificate verification
-    should be done. *)
+    This checks that the disk exists and that authentication is
+    correct, otherwise it will fail. *)
-- 
2.13.2

This is a small refactoring where we split get_session_cookie into a
function to fetch the URL and a function to parse the cookie.
---
 v2v/vCenter.ml | 106 ++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 59 insertions(+), 47 deletions(-)

diff --git a/v2v/vCenter.ml b/v2v/vCenter.ml
index 2962618d4..26c31131e 100644
--- a/v2v/vCenter.ml
+++ b/v2v/vCenter.ml
@@ -110,6 +110,45 @@ let rec map_source ?readahead ?password dcPath uri scheme
server path      sslverify = sslverify }
 
 and get_session_cookie password scheme uri sslverify https_url +  let status,
headers, dump_response +    fetch_headers_from_url password scheme uri sslverify
https_url in
+
+  if status = "401" then (
+    dump_response stderr;
+    if uri.uri_user <> None then
+      error (f_"vcenter: incorrect username or password")
+    else
+      error (f_"vcenter: incorrect username or password.  You might need
to specify the username in the URI like this: %s://USERNAME@[etc]")
+            scheme
+  );
+
+  if status = "404" then (
+    dump_response stderr;
+    error (f_"vcenter: URL not found: %s") https_url
+  );
+
+  if status <> "200" then (
+    dump_response stderr;
+    error (f_"vcenter: invalid response from server")
+  );
+
+  (* Get the cookie. *)
+  let rec loop = function
+    | [] ->
+       dump_response stderr;
+       warning (f_"vcenter: could not read session cookie from the vCenter
Server, conversion may consume all sessions on the server and fail part way
through");
+       None
+    | ("set-cookie", cookie) :: _ ->
+       let cookie, _ = String.split ";" cookie in
+       Some cookie
+
+    | _ :: headers ->
+       loop headers
+  in
+  loop headers
+
+(* Fetch the status and reply headers from a URL. *)
+and fetch_headers_from_url password scheme uri sslverify https_url    let
curl_args = ref [
     "head", None;
     "silent", None;
@@ -139,53 +178,26 @@ and get_session_cookie password scheme uri sslverify
https_url
   if verbose () then dump_response stderr;
 
+  let statuses, headers +    List.partition (
+      fun line ->
+        let len = String.length line in
+        len >= 12 && String.sub line 0 5 = "HTTP/"
+    ) lines in
+
   (* Look for the last HTTP/x.y NNN status code in the output. *)
-  let status = ref "" in
-  List.iter (
-    fun line ->
-      let len = String.length line in
-      if len >= 12 && String.sub line 0 5 = "HTTP/" then
-        status := String.sub line 9 3
-  ) lines;
-  let status = !status in
-  if status = "" then (
-    dump_response stderr;
-    error (f_"vcenter: no status code in output of ‘curl’ command.  Is
‘curl’ installed?")
-  );
-
-  if status = "401" then (
-    dump_response stderr;
-    if uri.uri_user <> None then
-      error (f_"vcenter: incorrect username or password")
-    else
-      error (f_"vcenter: incorrect username or password.  You might need
to specify the username in the URI like this: %s://USERNAME@[etc]")
-            scheme
-  );
-
-  if status = "404" then (
-    dump_response stderr;
-    error (f_"vcenter: URL not found: %s") https_url
-  );
-
-  if status <> "200" then (
-    dump_response stderr;
-    error (f_"vcenter: invalid response from server")
-  );
-
-  (* Get the cookie. *)
-  let rec loop = function
+  let status +    match statuses with
     | [] ->
        dump_response stderr;
-       warning (f_"vcenter: could not read session cookie from the vCenter
Server, conversion may consume all sessions on the server and fail part way
through");
-       None
-    | line :: lines ->
-       let len = String.length line in
-       if len >= 12 && String.sub line 0 12 = "Set-Cookie:
" then (
-         let line = String.sub line 12 (len-12) in
-         let cookie, _ = String.split ";" line in
-         Some cookie
-       )
-       else
-         loop lines
-  in
-  loop lines
+       error (f_"vcenter: no status code in output of ‘curl’ command.  Is
‘curl’ installed?")
+    | ss -> String.sub (List.hd (List.rev ss)) 9 3 in
+
+  let headers +    List.map (
+      fun header ->
+        let h, c = String.split ": " header in
+        String.lowercase_ascii h, c
+    ) headers in
+
+  status, headers, dump_response
-- 
2.13.2

Pure refactoring.
---
 v2v/vCenter.ml | 44 +++++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/v2v/vCenter.ml b/v2v/vCenter.ml
index 26c31131e..c96ebdb8b 100644
--- a/v2v/vCenter.ml
+++ b/v2v/vCenter.ml
@@ -45,27 +45,7 @@ let rec map_source ?readahead ?password dcPath uri scheme
server path         (* XXX only works if the query string is not URI-quoted *)
        String.find query "no_verify=1" = -1 in
 
-  let https_url -    if not (PCRE.matches source_re path) then
-      path
-    else (
-      let datastore = PCRE.sub 1 and path = PCRE.sub 2 in
-
-      let port -        match uri.uri_port with
-        | 443 -> ""
-        | n when n >= 1 -> ":" ^ string_of_int n
-        | _ -> "" in
-
-      (* XXX Old virt-v2v could also handle snapshots, ie:
-       * "[datastore1] Fedora 20/Fedora 20-NNNNNN.vmdk"
-       * XXX Need to handle templates.  The file is called
"-delta.vmdk" in
-       * place of "-flat.vmdk".
-       *)
-      sprintf
"https://%s%s/folder/%s-flat.vmdk?dcPath=%s&dsName=%s"
-              server port
-              (uri_quote path) (uri_quote dcPath) (uri_quote datastore)
-    ) in
+  let https_url = get_https_url dcPath uri server path in
 
   let session_cookie      get_session_cookie password scheme uri sslverify
https_url in
@@ -109,6 +89,28 @@ let rec map_source ?readahead ?password dcPath uri scheme
server path      session_cookie = session_cookie;
     sslverify = sslverify }
 
+and get_https_url dcPath uri server path +  if not (PCRE.matches source_re
path) then
+    path
+  else (
+    let datastore = PCRE.sub 1 and path = PCRE.sub 2 in
+
+    let port +      match uri.uri_port with
+      | 443 -> ""
+      | n when n >= 1 -> ":" ^ string_of_int n
+      | _ -> "" in
+
+    (* XXX Old virt-v2v could also handle snapshots, ie:
+     * "[datastore1] Fedora 20/Fedora 20-NNNNNN.vmdk"
+     * XXX Need to handle templates.  The file is called
"-delta.vmdk" in
+     * place of "-flat.vmdk".
+     *)
+    sprintf
"https://%s%s/folder/%s-flat.vmdk?dcPath=%s&dsName=%s"
+            server port
+            (uri_quote path) (uri_quote dcPath) (uri_quote datastore)
+  )
+
 and get_session_cookie password scheme uri sslverify https_url    let status,
headers, dump_response      fetch_headers_from_url password scheme uri sslverify
https_url in
-- 
2.13.2

This implements a missing feature from old virt-v2v, namely being able
to cope with a guest with snapshots.  Note this only converts the top
(latest) snapshot.  As in old virt-v2v it does NOT convert the whole
chain of snapshots.
---
 v2v/vCenter.ml | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/v2v/vCenter.ml b/v2v/vCenter.ml
index c96ebdb8b..8e0795c3c 100644
--- a/v2v/vCenter.ml
+++ b/v2v/vCenter.ml
@@ -33,6 +33,7 @@ type remote_resource = {
 }
 
 let source_re = PCRE.compile "^\\[(.*)\\] (.*)\\.vmdk$"
+let snapshot_re = PCRE.compile "^(.*)-\\d+(\\.vmdk)$"
 
 let rec map_source ?readahead ?password dcPath uri scheme server path    (* If
no_verify=1 was passed in the libvirt URI, then we have to
@@ -45,7 +46,28 @@ let rec map_source ?readahead ?password dcPath uri scheme
server path         (* XXX only works if the query string is not URI-quoted *)
        String.find query "no_verify=1" = -1 in
 
-  let https_url = get_https_url dcPath uri server path in
+  let https_url +    let https_url = get_https_url dcPath uri server path in
+    (* Check the URL exists. *)
+    let status, _, _ +      fetch_headers_from_url password scheme uri
sslverify https_url in
+    (* If a disk is actually a snapshot image it will have '-00000n'
+     * appended to its name, e.g.:
+     *   [yellow:storage1] RHEL4-X/RHEL4-X-000003.vmdk
+     * The flat storage is still called RHEL4-X-flat, however. If we got
+     * a 404 and the vmdk name looks like it might be a snapshot, try
+     * again without the snapshot suffix.
+     *)
+    if status = "404" && PCRE.matches snapshot_re path then (
+      let path = PCRE.sub 1 ^ PCRE.sub 2 in
+      get_https_url dcPath uri server path
+    )
+    else
+      (* Note that other non-200 status errors will be handled
+       * in get_session_cookie below, so we don't have to worry
+       * about them here.
+       *)
+      https_url in
 
   let session_cookie      get_session_cookie password scheme uri sslverify
https_url in
@@ -101,9 +123,7 @@ and get_https_url dcPath uri server path        | n when n
>= 1 -> ":" ^ string_of_int n
       | _ -> "" in
 
-    (* XXX Old virt-v2v could also handle snapshots, ie:
-     * "[datastore1] Fedora 20/Fedora 20-NNNNNN.vmdk"
-     * XXX Need to handle templates.  The file is called
"-delta.vmdk" in
+    (* XXX Need to handle templates.  The file is called
"-delta.vmdk" in
      * place of "-flat.vmdk".
      *)
     sprintf
"https://%s%s/folder/%s-flat.vmdk?dcPath=%s&dsName=%s"
-- 
2.13.2

On Friday, 13 October 2017 18:27:17 CEST Richard W.M. Jones
wrote:
> diff --git a/v2v/copy_to_local.ml b/v2v/copy_to_local.ml
> index 8a64f3a58..63e1ea310 100644
> --- a/v2v/copy_to_local.ml
> +++ b/v2v/copy_to_local.ml
> @@ -245,8 +245,10 @@ and parse_libvirt_xml guest_name xml >             
"vmware" "http://libvirt.org/schemas/domain/vmware/1.0";
>    let xpath_string = xpath_string xpathctx in
>  
> -  (* Get the dcpath, only present for libvirt >= 1.2.20 so use a
> -   * sensible default for older versions.
> +  (* Get the dcpath, present in libvirt >= 1.2.20.
> +   * XXX Unfortunately when testing virt-v2v-copy-to-local against a
> +   * local libvirt test XML file, we cannot pass in the vmware namespace.
> +   * So we must keep the default here until we can resolve that somehow.
>     *)
>    let dcpath >      Option.default "ha-datacenter"
An option here could be to make parse_libvirt_xml return the
"string option" for the dcpath (instead of string), and enforce it
it exists only when, in main, the disks are mapped for an ESXi source.

Something like the following patch on top of this:

diff --git a/v2v/copy_to_local.ml b/v2v/copy_to_local.ml
index f22e1cd86..58ef9d238 100644
--- a/v2v/copy_to_local.ml
+++ b/v2v/copy_to_local.ml
@@ -145,6 +145,11 @@ read the man page virt-v2v-copy-to-local(1).
   let disks      match source with
     | ESXi server ->
+       let dcpath +         match dcpath with
+         | Some dcpath -> dcpath
+         | None ->
+            error (f_"vcenter: <vmware:datacenterpath> was not found
in the XML.  You need to upgrade to libvirt ≥ 1.2.20.") in
        List.map (
          fun (remote_disk, local_disk) ->
            let url, sslverify @@ -245,14 +250,8 @@ and parse_libvirt_xml
guest_name xml                          "vmware"
"http://libvirt.org/schemas/domain/vmware/1.0";
   let xpath_string = xpath_string xpathctx in
 
-  (* Get the dcpath, present in libvirt >= 1.2.20.
-   * XXX Unfortunately when testing virt-v2v-copy-to-local against a
-   * local libvirt test XML file, we cannot pass in the vmware namespace.
-   * So we must keep the default here until we can resolve that somehow.
-   *)
-  let dcpath -    Option.default "ha-datacenter"
-                   (xpath_string "/domain/vmware:datacenterpath") in
+  (* Get the dcpath, present in libvirt >= 1.2.20. *)
+  let dcpath = xpath_string "/domain/vmware:datacenterpath" in
 
   (* Parse the disks. *)
   let get_disks, add_disk 
-- 
Pino Toscano

On Friday, 13 October 2017 18:27:21 CEST Richard W.M. Jones
wrote:
> diff --git a/v2v/vCenter.ml b/v2v/vCenter.ml
> index c96ebdb8b..8e0795c3c 100644
> --- a/v2v/vCenter.ml
> +++ b/v2v/vCenter.ml
> @@ -33,6 +33,7 @@ type remote_resource = {
>  }
>  
>  let source_re = PCRE.compile "^\\[(.*)\\] (.*)\\.vmdk$"
> +let snapshot_re = PCRE.compile "^(.*)-\\d+(\\.vmdk)$"
>  
>  let rec map_source ?readahead ?password dcPath uri scheme server path >
(* If no_verify=1 was passed in the libvirt URI, then we have to
> @@ -45,7 +46,28 @@ let rec map_source ?readahead ?password dcPath uri
scheme server path >         (* XXX only works if the query string is not
URI-quoted *)
>         String.find query "no_verify=1" = -1 in
>  
> -  let https_url = get_https_url dcPath uri server path in
> +  let https_url > +    let https_url = get_https_url dcPath uri server
path in
> +    (* Check the URL exists. *)
> +    let status, _, _ > +      fetch_headers_from_url password scheme
uri sslverify https_url in
> +    (* If a disk is actually a snapshot image it will have
'-00000n'
> +     * appended to its name, e.g.:
> +     *   [yellow:storage1] RHEL4-X/RHEL4-X-000003.vmdk
> +     * The flat storage is still called RHEL4-X-flat, however. If we got
> +     * a 404 and the vmdk name looks like it might be a snapshot, try
> +     * again without the snapshot suffix.
> +     *)
> +    if status = "404" && PCRE.matches snapshot_re path
then (
> +      let path = PCRE.sub 1 ^ PCRE.sub 2 in
> +      get_https_url dcPath uri server path
> +    )
IMHO snapshot_re is a bit too generic, and it matches also guests named
like "fedora-26" or so.  If the '-00000n' suffix is always
made by
6 digits, then maybe "^(.*)-\\d{6}(\\.vmdk)$" avoids a number of false
positives.

-- 
Pino Toscano