Libguestfs - Jun 2018 - [PATCH 0/3] v2v: Various refactorings.

Use -ip instead of --password-file, and various refactorings.

It strikes me that we should probably deprecate and eventually remove
virt-v2v-copy-to-local.  With the introduction of the new SSH and VDDK
transports, and with RHEL 5 Xen becoming more irrelevant, it's no
longer needed.

Rich.

---
 v2v/copy_to_local.ml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/v2v/copy_to_local.ml b/v2v/copy_to_local.ml
index 5992689bc..8ee9145b9 100644
--- a/v2v/copy_to_local.ml
+++ b/v2v/copy_to_local.ml
@@ -44,7 +44,7 @@ let rec main ()  
   (* Handle the command line. *)
   let argspec = [
-    [ L"ic" ],       Getopt.String ("uri",
set_string_option_once "-ic" input_conn),
+    [ M"ic" ],       Getopt.String ("uri",
set_string_option_once "-ic" input_conn),
                                             s_"Libvirt URI";
     [ L"password-file" ], Getopt.String ("file",
set_string_option_once "--password-file" password_file),
                                             s_"Use password from
file";
-- 
2.16.2

Consistent with the option -op added in
commit a4e181137a38f5767dd1bf05dc482959cb7283be, use -ip to pass
passwords for the input side.

This replaces --password-file, and like that option you have to pass a
filename.

This also changes the code so we pass around the filename instead of
the password between functions.  For ‘-it vddk’ this simplifies
things: we can pass the filename through to nbdkit and we never need
to read it in virt-v2v.  For other input methods we eventually need to
read the password from the file at some lower level place in the code.
---
 v2v/cmdline.ml                     | 21 ++++++++-------------
 v2v/copy_to_local.ml               | 12 +++---------
 v2v/input_libvirt.ml               | 14 +++++++-------
 v2v/input_libvirt.mli              |  2 +-
 v2v/input_libvirt_other.ml         |  9 +++++----
 v2v/input_libvirt_vcenter_https.ml | 11 ++++++-----
 v2v/input_libvirt_vddk.ml          | 25 +++++++++----------------
 v2v/input_libvirt_vddk.mli         |  2 +-
 v2v/input_libvirt_xen_ssh.ml       |  7 ++++---
 v2v/libvirt_utils.ml               |  6 ++++++
 v2v/libvirt_utils.mli              |  4 ++--
 v2v/test-v2v-docs.sh               |  2 ++
 v2v/vCenter.ml                     | 20 +++++++++++---------
 v2v/vCenter.mli                    |  4 ++--
 v2v/virt-v2v-copy-to-local.pod     |  2 +-
 v2v/virt-v2v.pod                   | 19 ++++++++++---------
 16 files changed, 78 insertions(+), 82 deletions(-)

diff --git a/v2v/cmdline.ml b/v2v/cmdline.ml
index a8da63f71..5414029d5 100644
--- a/v2v/cmdline.ml
+++ b/v2v/cmdline.ml
@@ -59,6 +59,7 @@ let parse_cmdline ()  
   let input_conn = ref None in
   let input_format = ref None in
+  let input_password = ref None in
   let input_transport = ref None in
   let in_place = ref false in
   let output_conn = ref None in
@@ -66,7 +67,6 @@ let parse_cmdline ()    let output_name = ref None in
   let output_password = ref None in
   let output_storage = ref None in
-  let password_file = ref None in
 
   let set_string_option_once optname optref arg      match !optref with
@@ -198,6 +198,8 @@ let parse_cmdline ()                                     
s_"Input format (for -i disk)";
     [ M"io" ],       Getopt.String ("option[=value]",
set_input_option),
                                     s_"Set option for input mode";
+    [ M"ip" ],       Getopt.String ("filename",
set_string_option_once "-ip" input_password),
+                                    s_"Use password from file to connect
to input hypervisor";
     [ M"it" ],       Getopt.String ("transport",
set_string_option_once "-it" input_transport),
                                     s_"Input transport";
     [ L"in-place" ], Getopt.Set in_place,
@@ -226,8 +228,8 @@ let parse_cmdline ()                                     
s_"Use password from file to connect to output hypervisor";
     [ M"os" ],       Getopt.String ("storage",
set_string_option_once "-os" output_storage),
                                     s_"Set output storage location";
-    [ L"password-file" ], Getopt.String ("file",
set_string_option_once "--password-file" password_file),
-                                    s_"Use password from file";
+    [ L"password-file" ], Getopt.String ("filename",
set_string_option_once "--password-file" input_password),
+                                    s_"Same as ‘-ip filename’";
     [ L"print-source" ], Getopt.Set print_source,
                                     s_"Print source and stop";
     [ L"print-target" ], Getopt.Set print_target,
@@ -304,6 +306,7 @@ read the man page virt-v2v(1).
   let input_format = !input_format in
   let input_mode = !input_mode in
   let input_options = List.rev !input_options in
+  let input_password = !input_password in
   let input_transport      match !input_transport with
     | None -> None
@@ -325,7 +328,6 @@ read the man page virt-v2v(1).
   let output_options = List.rev !output_options in
   let output_password = !output_password in
   let output_storage = !output_storage in
-  let password_file = !password_file in
   let print_source = !print_source in
   let print_target = !print_target in
   let qemu_boot = !qemu_boot in
@@ -426,14 +428,6 @@ read the man page virt-v2v(1).
       error (f_"--in-place and --print-target cannot be used
together")
   );
 
-  (* Parse out the password from the password file. *)
-  let password -    match password_file with
-    | None -> None
-    | Some filename ->
-      let password = read_first_line_from_file filename in
-      Some password in
-
   (* Parsing of the argument(s) depends on the input mode. *)
   let input      match input_mode with
@@ -462,7 +456,8 @@ read the man page virt-v2v(1).
         | (Some (`VDDK _) as vddk) -> vddk
         | Some `SSH ->
            error (f_"only ‘-it vddk’ can be used here") in
-      Input_libvirt.input_libvirt password input_conn input_transport guest
+      Input_libvirt.input_libvirt input_password input_conn input_transport
+                                  guest
 
     | `LibvirtXML ->
       (* -i libvirtxml: Expecting a filename (XML file). *)
diff --git a/v2v/copy_to_local.ml b/v2v/copy_to_local.ml
index 8ee9145b9..c626b6368 100644
--- a/v2v/copy_to_local.ml
+++ b/v2v/copy_to_local.ml
@@ -46,6 +46,8 @@ let rec main ()    let argspec = [
     [ M"ic" ],       Getopt.String ("uri",
set_string_option_once "-ic" input_conn),
                                             s_"Libvirt URI";
+    [ M"ip" ],       Getopt.String ("file",
set_string_option_once "-ip" password_file),
+                                            s_"Use password from
file";
     [ L"password-file" ], Getopt.String ("file",
set_string_option_once "--password-file" password_file),
                                             s_"Use password from
file";
   ] in
@@ -86,14 +88,6 @@ read the man page virt-v2v-copy-to-local(1).
        error (f_"the -ic parameter is required") (* at the moment *)
     | Some ic -> ic in
 
-  (* Parse out the password from the password file. *)
-  let password -    match password_file with
-    | None -> None
-    | Some filename ->
-      let password = read_first_line_from_file filename in
-      Some password in
-
   (* Check this is a libvirt URI we can understand. *)
   let parsed_uri      try Xml.parse_uri input_conn
@@ -131,7 +125,7 @@ read the man page virt-v2v-copy-to-local(1).
 
   (* Get the remote libvirt XML. *)
   message (f_"Fetching the remote libvirt XML metadata ...");
-  let xml = Libvirt_utils.dumpxml ?password ~conn:input_conn guest_name in
+  let xml = Libvirt_utils.dumpxml ?password_file ~conn:input_conn guest_name in
 
   debug "libvirt XML from remote server:\n%s" xml;
 
diff --git a/v2v/input_libvirt.ml b/v2v/input_libvirt.ml
index 377257dc2..23ca0e281 100644
--- a/v2v/input_libvirt.ml
+++ b/v2v/input_libvirt.ml
@@ -27,10 +27,10 @@ open Types
 open Utils
 
 (* Choose the right subclass based on the URI. *)
-let input_libvirt password libvirt_uri input_transport guest +let input_libvirt
input_password libvirt_uri input_transport guest    match libvirt_uri with
   | None ->
-    Input_libvirt_other.input_libvirt_other password libvirt_uri guest
+    Input_libvirt_other.input_libvirt_other input_password libvirt_uri guest
 
   | Some orig_uri ->
     let { Xml.uri_server = server; uri_scheme = scheme } as parsed_uri @@
-45,22 +45,22 @@ let input_libvirt password libvirt_uri input_transport guest
     | Some _, None, _                   (* No scheme? *)
     | Some _, Some "", _ ->
-      Input_libvirt_other.input_libvirt_other password libvirt_uri guest
+      Input_libvirt_other.input_libvirt_other input_password libvirt_uri guest
 
     (* vCenter over https. *)
     | Some server, Some ("esx"|"gsx"|"vpx"), None
->
        Input_libvirt_vcenter_https.input_libvirt_vcenter_https
-         password libvirt_uri parsed_uri server guest
+         input_password libvirt_uri parsed_uri server guest
 
     (* vCenter or ESXi using nbdkit vddk plugin *)
     | Some server, Some ("esx"|"gsx"|"vpx"), Some
(`VDDK vddk_options) ->
-       Input_libvirt_vddk.input_libvirt_vddk vddk_options password
+       Input_libvirt_vddk.input_libvirt_vddk vddk_options input_password
                                              libvirt_uri parsed_uri guest
 
     (* Xen over SSH *)
     | Some server, Some "xen+ssh", _ ->
       Input_libvirt_xen_ssh.input_libvirt_xen_ssh
-        password libvirt_uri parsed_uri server guest
+        input_password libvirt_uri parsed_uri server guest
 
     (* Old virt-v2v also supported qemu+ssh://.  However I am
      * deliberately not supporting this in new virt-v2v.  Don't
@@ -71,6 +71,6 @@ let input_libvirt password libvirt_uri input_transport guest  
| Some _, Some _, _ ->
       warning (f_"no support for remote libvirt connections to '-ic
%s'.  The conversion may fail when it tries to read the source disks.")
         orig_uri;
-      Input_libvirt_other.input_libvirt_other password libvirt_uri guest
+      Input_libvirt_other.input_libvirt_other input_password libvirt_uri guest
 
 let () = Modules_list.register_input_module "libvirt"
diff --git a/v2v/input_libvirt.mli b/v2v/input_libvirt.mli
index 08824bb67..7b486532b 100644
--- a/v2v/input_libvirt.mli
+++ b/v2v/input_libvirt.mli
@@ -19,6 +19,6 @@
 (** [-i libvirt] source. *)
 
 val input_libvirt : string option -> string option -> [`VDDK of
Input_libvirt_vddk.vddk_options] option -> string -> Types.input
-(** [input_libvirt password libvirt_uri input_transport guest]
+(** [input_libvirt input_password libvirt_uri input_transport guest]
     creates and returns a new {!Types.input} object specialized for reading
     input from libvirt sources. *)
diff --git a/v2v/input_libvirt_other.ml b/v2v/input_libvirt_other.ml
index 6ffca3e44..25714d2c3 100644
--- a/v2v/input_libvirt_other.ml
+++ b/v2v/input_libvirt_other.ml
@@ -40,7 +40,7 @@ let error_if_libvirt_does_not_support_json_backingfile ()     
error (f_"because of libvirt bug https://bugzilla.redhat.com/1134878 you
must EITHER upgrade to libvirt >= 2.1.0 OR set this environment
variable:\n\nexport LIBGUESTFS_BACKEND=direct\n\nand then rerun the virt-v2v
command.")
 
 (* Superclass. *)
-class virtual input_libvirt (password : string option) libvirt_uri guest +class
virtual input_libvirt (input_password : string option) libvirt_uri guest  object
   inherit input
 
@@ -55,9 +55,9 @@ end
 (* Subclass specialized for handling anything that's *not* VMware vCenter
  * or Xen.
  *)
-class input_libvirt_other password libvirt_uri guest +class input_libvirt_other
input_password libvirt_uri guest  object
-  inherit input_libvirt password libvirt_uri guest
+  inherit input_libvirt input_password libvirt_uri guest
 
   method source ()      debug "input_libvirt_other: source ()";
@@ -65,7 +65,8 @@ object
     (* Get the libvirt XML.  This also checks (as a side-effect)
      * that the domain is not running.  (RHBZ#1138586)
      *)
-    let xml = Libvirt_utils.dumpxml ?password ?conn:libvirt_uri guest in
+    let xml = Libvirt_utils.dumpxml ?password_file:input_password
+                                    ?conn:libvirt_uri guest in
 
     let source, disks = parse_libvirt_xml ?conn:libvirt_uri xml in
     let disks = List.map (fun { p_source_disk = disk } -> disk) disks in
diff --git a/v2v/input_libvirt_vcenter_https.ml
b/v2v/input_libvirt_vcenter_https.ml
index e0c84ddfc..dfe3dcf96 100644
--- a/v2v/input_libvirt_vcenter_https.ml
+++ b/v2v/input_libvirt_vcenter_https.ml
@@ -36,9 +36,9 @@ let readahead_for_copying = Some (64 * 1024 * 1024)
 
 (* Subclass specialized for handling VMware vCenter over https. *)
 class input_libvirt_vcenter_https
-        password libvirt_uri parsed_uri server guest +        input_password
libvirt_uri parsed_uri server guest  object
-  inherit input_libvirt password libvirt_uri guest
+  inherit input_libvirt input_password libvirt_uri guest
 
   val saved_source_paths = Hashtbl.create 13
   val mutable dcPath = ""
@@ -64,7 +64,8 @@ object
     (* Get the libvirt XML.  This also checks (as a side-effect)
      * that the domain is not running.  (RHBZ#1138586)
      *)
-    let xml = Libvirt_utils.dumpxml ?password ?conn:libvirt_uri guest in
+    let xml = Libvirt_utils.dumpxml ?password_file:input_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.  This
@@ -102,7 +103,7 @@ object
       | { p_source_disk = disk; p_source = P_dont_rewrite } -> disk
       | { p_source_disk = disk; p_source = P_source_file path } ->
         let { VCenter.qemu_uri } -          VCenter.map_source ?readahead
?password
+          VCenter.map_source ?readahead ?password_file:input_password
                              dcPath parsed_uri server path in
 
         (* The libvirt ESX driver doesn't normally specify a format, but
@@ -123,7 +124,7 @@ object
     | Some orig_path ->
       let readahead = readahead_for_copying in
       let { VCenter.qemu_uri = backing_qemu_uri } -        VCenter.map_source
?readahead ?password
+        VCenter.map_source ?readahead ?password_file:input_password
                            dcPath parsed_uri server orig_path in
 
       (* Rebase the qcow2 overlay to adjust the readahead parameter. *)
diff --git a/v2v/input_libvirt_vddk.ml b/v2v/input_libvirt_vddk.ml
index 0b3ed7af9..d0633d958 100644
--- a/v2v/input_libvirt_vddk.ml
+++ b/v2v/input_libvirt_vddk.ml
@@ -95,7 +95,8 @@ let parse_input_options options    options
 
 (* Subclass specialized for handling VMware via nbdkit vddk plugin. *)
-class input_libvirt_vddk vddk_options password libvirt_uri parsed_uri guest
+class input_libvirt_vddk vddk_options input_password libvirt_uri parsed_uri
+                         guest    (* The VDDK path. *)
   let libdir      try Some (List.assoc "libdir" vddk_options)
@@ -199,7 +200,7 @@ See also \"INPUT FROM VDDK\" in the virt-v2v(1)
manual.") libNN
   in
 
 object
-  inherit input_libvirt password libvirt_uri guest as super
+  inherit input_libvirt input_password libvirt_uri guest as super
 
   method precheck ()      error_unless_vddk_libdir ();
@@ -222,7 +223,8 @@ object
     (* Get the libvirt XML.  This also checks (as a side-effect)
      * that the domain is not running.  (RHBZ#1138586)
      *)
-    let xml = Libvirt_utils.dumpxml ?password ?conn:libvirt_uri guest in
+    let xml = Libvirt_utils.dumpxml ?password_file:input_password
+                                    ?conn:libvirt_uri guest in
     let source, disks = parse_libvirt_xml ?conn:libvirt_uri xml in
 
     (* Find the <vmware:moref> element from the XML.  This was added
@@ -239,15 +241,12 @@ object
       | None ->
          error (f_"<vmware:moref> was not found in the output of
‘virsh dumpxml \"%s\"’.  The most likely reason is that libvirt is too
old, try upgrading libvirt to ≥ 3.7.") guest in
 
-    (* Create a temporary directory where we place the sockets and
-     * password file.
-     *)
+    (* Create a temporary directory where we place the sockets. *)
     let tmpdir        let base_dir = (open_guestfs ())#get_cachedir () in
       let t = Mkdtemp.temp_dir ~base_dir "vddk." in
       (* tmpdir must be readable (but not writable) by "other" so
that
-       * qemu can open the sockets.  If we place a password file in
-       * this directory then we'll chmod that to 0600 below.
+       * qemu can open the sockets.
        *)
       chmod t 0o755;
       rmdir_on_exit t;
@@ -299,17 +298,11 @@ object
       add_arg "vddk";
 
       let password_param -        match password with
+        match input_password with
         | None ->
            (* nbdkit asks for the password interactively *)
            "password=-"
-        | Some password ->
-           let password_file = tmpdir // "password" in
-           with_open_out password_file (
-             fun chan ->
-               fchmod (descr_of_out_channel chan) 0o600;
-               output_string chan password
-           );
+        | Some password_file ->
            (* nbdkit reads the password from the file *)
            "password=+" ^ password_file in
       add_arg (sprintf "server=%s" server);
diff --git a/v2v/input_libvirt_vddk.mli b/v2v/input_libvirt_vddk.mli
index 1cebba506..ef975c280 100644
--- a/v2v/input_libvirt_vddk.mli
+++ b/v2v/input_libvirt_vddk.mli
@@ -26,6 +26,6 @@ val parse_input_options : (string * string) list ->
vddk_options
 (** Print and parse vddk -io options. *)
 
 val input_libvirt_vddk : vddk_options -> string option -> string option
-> Xml.uri -> string -> Types.input
-(** [input_libvirt_vddk vddk_options password libvirt_uri parsed_uri guest]
+(** [input_libvirt_vddk vddk_options input_password libvirt_uri parsed_uri
guest]
     creates and returns a {!Types.input} object specialized for reading
     the guest disks using the nbdkit vddk plugin. *)
diff --git a/v2v/input_libvirt_xen_ssh.ml b/v2v/input_libvirt_xen_ssh.ml
index 22a58396c..2e769cd0b 100644
--- a/v2v/input_libvirt_xen_ssh.ml
+++ b/v2v/input_libvirt_xen_ssh.ml
@@ -30,9 +30,9 @@ open Input_libvirt_other
 open Printf
 
 (* Subclass specialized for handling Xen over SSH. *)
-class input_libvirt_xen_ssh password libvirt_uri parsed_uri server guest +class
input_libvirt_xen_ssh input_password libvirt_uri parsed_uri server guest  object
-  inherit input_libvirt password libvirt_uri guest
+  inherit input_libvirt input_password libvirt_uri guest
 
   method precheck ()      if backend_is_libvirt () then
@@ -46,7 +46,8 @@ object
     (* Get the libvirt XML.  This also checks (as a side-effect)
      * that the domain is not running.  (RHBZ#1138586)
      *)
-    let xml = Libvirt_utils.dumpxml ?password ?conn:libvirt_uri guest in
+    let xml = Libvirt_utils.dumpxml ?password_file:input_password
+                                    ?conn:libvirt_uri guest in
     let source, disks = parse_libvirt_xml ?conn:libvirt_uri xml in
 
     (* Map the <source/> filename (which is relative to the remote
diff --git a/v2v/libvirt_utils.ml b/v2v/libvirt_utils.ml
index efedca811..1f5eb712a 100644
--- a/v2v/libvirt_utils.ml
+++ b/v2v/libvirt_utils.ml
@@ -16,10 +16,16 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  *)
 
+open Std_utils
+
 (* This module implements various [virsh]-like commands, but with
     non-broken authentication handling. *)
 
 external dumpxml : ?password:string -> ?conn:string -> string ->
string = "v2v_dumpxml"
+let dumpxml ?password_file +  let password = Option.map
read_first_line_from_file password_file in
+  dumpxml ?password
+
 external pool_dumpxml : ?conn:string -> string -> string =
"v2v_pool_dumpxml"
 external vol_dumpxml : ?conn:string -> string -> string -> string =
"v2v_vol_dumpxml"
 
diff --git a/v2v/libvirt_utils.mli b/v2v/libvirt_utils.mli
index 63a0af3af..bb65a4283 100644
--- a/v2v/libvirt_utils.mli
+++ b/v2v/libvirt_utils.mli
@@ -24,8 +24,8 @@
     password prompt to stdout, which is the same place we would be
     reading the XML from.  This file works around this brokenness. *)
 
-val dumpxml : ?password:string -> ?conn:string -> string -> string
-(** [dumpxml ?password ?conn dom] returns the libvirt XML of domain [dom].
+val dumpxml : ?password_file:string -> ?conn:string -> string ->
string
+(** [dumpxml ?password_file ?conn dom] returns the libvirt XML of domain [dom].
     The optional [?conn] parameter is the libvirt connection URI.
     [dom] may be a guest name or UUID. *)
 
diff --git a/v2v/test-v2v-docs.sh b/v2v/test-v2v-docs.sh
index e1e22b599..64dfe5492 100755
--- a/v2v/test-v2v-docs.sh
+++ b/v2v/test-v2v-docs.sh
@@ -27,8 +27,10 @@ $top_srcdir/podcheck.pl virt-v2v.pod virt-v2v \
 --ic,\
 --if,\
 --io,\
+--ip,\
 --it,\
 --no-trim,\
+--password-file,\
 --oa,\
 --oc,\
 --of,\
diff --git a/v2v/vCenter.ml b/v2v/vCenter.ml
index ddb8076f4..e97d25ce1 100644
--- a/v2v/vCenter.ml
+++ b/v2v/vCenter.ml
@@ -35,7 +35,7 @@ type remote_resource = {
 let source_re = PCRE.compile "^\\[(.*)\\] (.*)\\.vmdk$"
 let snapshot_re = PCRE.compile "^(.*)-\\d{6}(\\.vmdk)$"
 
-let rec map_source ?readahead ?password dcPath uri server path +let rec
map_source ?readahead ?password_file dcPath uri server path    (* If no_verify=1
was passed in the libvirt URI, then we have to
    * turn off certificate verification here too.
    *)
@@ -50,7 +50,7 @@ let rec map_source ?readahead ?password dcPath uri server path
let https_url = get_https_url dcPath uri server path in
     (* Check the URL exists. *)
     let status, _, _ -      fetch_headers_from_url password uri sslverify
https_url in
+      fetch_headers_from_url password_file 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
@@ -69,7 +69,8 @@ let rec map_source ?readahead ?password dcPath uri server path
*)
       https_url in
 
-  let session_cookie = get_session_cookie password uri sslverify https_url in
+  let session_cookie +    get_session_cookie password_file uri sslverify
https_url in
 
   let qemu_uri      (* Construct the JSON parameters for the qemu URI. *)
@@ -130,9 +131,9 @@ and get_https_url dcPath uri server path             
(uri_quote path) (uri_quote dcPath) (uri_quote datastore)
   )
 
-and get_session_cookie password uri sslverify https_url +and get_session_cookie
password_file uri sslverify https_url    let status, headers, dump_response -   
fetch_headers_from_url password uri sslverify https_url in
+    fetch_headers_from_url password_file uri sslverify https_url in
 
   if status = "401" then (
     dump_response stderr;
@@ -168,19 +169,20 @@ and get_session_cookie password uri sslverify https_url   
loop headers
 
 (* Fetch the status and reply headers from a URL. *)
-and fetch_headers_from_url password uri sslverify https_url +and
fetch_headers_from_url password_file uri sslverify https_url    let curl_args =
ref [
     "head", None;
     "silent", None;
     "url", Some https_url;
   ] in
-  (match uri.uri_user, password with
+  (match uri.uri_user, password_file with
    | None, None -> ()
    | None, Some _ ->
-      warning (f_"--password-file parameter ignored because
'user@' was not given in the URL")
+      warning (f_"-ip 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 ->
+   | Some user, Some password_file ->
+      let password = read_first_line_from_file password_file in
       List.push_back curl_args ("user", Some (user ^ ":" ^
password))
   );
   if not sslverify then List.push_back curl_args ("insecure", None);
diff --git a/v2v/vCenter.mli b/v2v/vCenter.mli
index dbca95ffd..fd1017aaf 100644
--- a/v2v/vCenter.mli
+++ b/v2v/vCenter.mli
@@ -54,8 +54,8 @@ type remote_resource = {
 (** The "remote resource" is the structure returned by the
{!map_source}
     function. *)
 
-val map_source : ?readahead:int -> ?password:string -> string ->
Xml.uri -> string -> string -> remote_resource
-(** [map_source ?readahead ?password dcPath uri server path]
+val map_source : ?readahead:int -> ?password_file:string -> string ->
Xml.uri -> string -> string -> remote_resource
+(** [map_source ?readahead ?password_file dcPath uri 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.
diff --git a/v2v/virt-v2v-copy-to-local.pod b/v2v/virt-v2v-copy-to-local.pod
index e5a552349..e06391757 100644
--- a/v2v/virt-v2v-copy-to-local.pod
+++ b/v2v/virt-v2v-copy-to-local.pod
@@ -102,7 +102,7 @@ Display help.
 
 Specify a libvirt connection URI
 
-=item B<--password-file> file
+=item B<-ip> file
 
 Instead of asking for password(s) interactively, pass the password
 through a file.  Note the file should contain the whole password,
diff --git a/v2v/virt-v2v.pod b/v2v/virt-v2v.pod
index b23fac5c8..251b4919f 100644
--- a/v2v/virt-v2v.pod
+++ b/v2v/virt-v2v.pod
@@ -440,6 +440,14 @@ When using VDDK mode, these options are passed unmodified
to the
 L<nbdkit(1)> VDDK plugin.  Please refer to
L<nbdkit-vddk-plugin(1)>.
 These are all optional.
 
+=item B<-ip> filename
+
+Supply a file containing a password to be used when connecting to the
+target hypervisor.  If this is omitted then the input hypervisor may
+ask for the password interactively.  Note the file should contain the
+whole password, B<without any trailing newline>, and for security the
+file should have mode C<0600> so that others cannot read it.
+
 =item B<-it> B<ssh>
 
 When using I<-i vmx>, this enables the ssh transport.
@@ -775,13 +783,6 @@ C<root>.
 You will get an error if virt-v2v is unable to mount/write to the
 Export Storage Domain.
 
-=item B<--password-file> file
-
-Instead of asking for password(s) interactively, pass the password
-through a file.  Note the file should contain the whole password,
-B<without any trailing newline>, and for security the file should have
-mode C<0600> so that others cannot read it.
-
 =item B<--print-source>
 
 Print information about the source guest and stop.  This option is
@@ -1291,8 +1292,8 @@ down).
 
 Note that you may be asked for the vCenter password I<twice>.  This
 happens once because libvirt needs it, and a second time because
-virt-v2v itself connects directly to the server.  Use
-I<--password-file> to supply a password via a file.
+virt-v2v itself connects directly to the server.  Use I<-ip>
+F<filename> to supply a password via a file.
 
 In this case the output flags are set to write the converted guest to
 a temporary directory as this is just an example, but you can also
-- 
2.16.2

Simple refactoring of the order of parameters to input objects
so they are always in the order:

  input_foo          ; class name
    input_conn       ; command line -iX parameters, alphabetically
    input_password   ;
    ...              ;
    vddk_options     ; extra class parameters
    ...              ;
    guestname        ; remaining command line parameters
---
 v2v/cmdline.ml                     |  2 +-
 v2v/input_libvirt.ml               | 18 +++++++++---------
 v2v/input_libvirt.mli              |  2 +-
 v2v/input_libvirt_other.ml         | 12 ++++++------
 v2v/input_libvirt_vcenter_https.ml |  8 ++++----
 v2v/input_libvirt_vddk.ml          | 14 +++++++-------
 v2v/input_libvirt_vddk.mli         |  4 ++--
 v2v/input_libvirt_xen_ssh.ml       |  8 ++++----
 8 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/v2v/cmdline.ml b/v2v/cmdline.ml
index 5414029d5..eaa11dba0 100644
--- a/v2v/cmdline.ml
+++ b/v2v/cmdline.ml
@@ -456,7 +456,7 @@ read the man page virt-v2v(1).
         | (Some (`VDDK _) as vddk) -> vddk
         | Some `SSH ->
            error (f_"only ‘-it vddk’ can be used here") in
-      Input_libvirt.input_libvirt input_password input_conn input_transport
+      Input_libvirt.input_libvirt input_conn input_password input_transport
                                   guest
 
     | `LibvirtXML ->
diff --git a/v2v/input_libvirt.ml b/v2v/input_libvirt.ml
index 23ca0e281..2803c00f9 100644
--- a/v2v/input_libvirt.ml
+++ b/v2v/input_libvirt.ml
@@ -27,10 +27,10 @@ open Types
 open Utils
 
 (* Choose the right subclass based on the URI. *)
-let input_libvirt input_password libvirt_uri input_transport guest -  match
libvirt_uri with
+let input_libvirt input_conn input_password input_transport guest +  match
input_conn with
   | None ->
-    Input_libvirt_other.input_libvirt_other input_password libvirt_uri guest
+    Input_libvirt_other.input_libvirt_other input_conn input_password guest
 
   | Some orig_uri ->
     let { Xml.uri_server = server; uri_scheme = scheme } as parsed_uri @@
-45,22 +45,22 @@ let input_libvirt input_password libvirt_uri input_transport
guest
     | Some _, None, _                   (* No scheme? *)
     | Some _, Some "", _ ->
-      Input_libvirt_other.input_libvirt_other input_password libvirt_uri guest
+      Input_libvirt_other.input_libvirt_other input_conn input_password guest
 
     (* vCenter over https. *)
     | Some server, Some ("esx"|"gsx"|"vpx"), None
->
        Input_libvirt_vcenter_https.input_libvirt_vcenter_https
-         input_password libvirt_uri parsed_uri server guest
+         input_conn input_password parsed_uri server guest
 
     (* vCenter or ESXi using nbdkit vddk plugin *)
     | Some server, Some ("esx"|"gsx"|"vpx"), Some
(`VDDK vddk_options) ->
-       Input_libvirt_vddk.input_libvirt_vddk vddk_options input_password
-                                             libvirt_uri parsed_uri guest
+       Input_libvirt_vddk.input_libvirt_vddk input_conn input_password
+                                             vddk_options parsed_uri guest
 
     (* Xen over SSH *)
     | Some server, Some "xen+ssh", _ ->
       Input_libvirt_xen_ssh.input_libvirt_xen_ssh
-        input_password libvirt_uri parsed_uri server guest
+        input_conn input_password parsed_uri server guest
 
     (* Old virt-v2v also supported qemu+ssh://.  However I am
      * deliberately not supporting this in new virt-v2v.  Don't
@@ -71,6 +71,6 @@ let input_libvirt input_password libvirt_uri input_transport
guest      | Some _, Some _, _ ->
       warning (f_"no support for remote libvirt connections to '-ic
%s'.  The conversion may fail when it tries to read the source disks.")
         orig_uri;
-      Input_libvirt_other.input_libvirt_other input_password libvirt_uri guest
+      Input_libvirt_other.input_libvirt_other input_conn input_password guest
 
 let () = Modules_list.register_input_module "libvirt"
diff --git a/v2v/input_libvirt.mli b/v2v/input_libvirt.mli
index 7b486532b..1a5e8f3a1 100644
--- a/v2v/input_libvirt.mli
+++ b/v2v/input_libvirt.mli
@@ -19,6 +19,6 @@
 (** [-i libvirt] source. *)
 
 val input_libvirt : string option -> string option -> [`VDDK of
Input_libvirt_vddk.vddk_options] option -> string -> Types.input
-(** [input_libvirt input_password libvirt_uri input_transport guest]
+(** [input_libvirt input_conn input_password input_transport guest]
     creates and returns a new {!Types.input} object specialized for reading
     input from libvirt sources. *)
diff --git a/v2v/input_libvirt_other.ml b/v2v/input_libvirt_other.ml
index 25714d2c3..1414fe4f9 100644
--- a/v2v/input_libvirt_other.ml
+++ b/v2v/input_libvirt_other.ml
@@ -40,13 +40,13 @@ let error_if_libvirt_does_not_support_json_backingfile ()   
error (f_"because of libvirt bug https://bugzilla.redhat.com/1134878 you
must EITHER upgrade to libvirt >= 2.1.0 OR set this environment
variable:\n\nexport LIBGUESTFS_BACKEND=direct\n\nand then rerun the virt-v2v
command.")
 
 (* Superclass. *)
-class virtual input_libvirt (input_password : string option) libvirt_uri guest
+class virtual input_libvirt input_conn (input_password : string option) guest 
object
   inherit input
 
   method as_options      sprintf "-i libvirt%s %s"
-      (match libvirt_uri with
+      (match input_conn with
       | None -> ""
       | Some uri -> " -ic " ^ uri)
       guest
@@ -55,9 +55,9 @@ end
 (* Subclass specialized for handling anything that's *not* VMware vCenter
  * or Xen.
  *)
-class input_libvirt_other input_password libvirt_uri guest +class
input_libvirt_other input_conn input_password guest  object
-  inherit input_libvirt input_password libvirt_uri guest
+  inherit input_libvirt input_conn input_password guest
 
   method source ()      debug "input_libvirt_other: source ()";
@@ -66,9 +66,9 @@ object
      * that the domain is not running.  (RHBZ#1138586)
      *)
     let xml = Libvirt_utils.dumpxml ?password_file:input_password
-                                    ?conn:libvirt_uri guest in
+                                    ?conn:input_conn guest in
 
-    let source, disks = parse_libvirt_xml ?conn:libvirt_uri xml in
+    let source, disks = parse_libvirt_xml ?conn:input_conn xml in
     let disks = List.map (fun { p_source_disk = disk } -> disk) disks in
     { source with s_disks = disks }
 end
diff --git a/v2v/input_libvirt_vcenter_https.ml
b/v2v/input_libvirt_vcenter_https.ml
index dfe3dcf96..edde185cd 100644
--- a/v2v/input_libvirt_vcenter_https.ml
+++ b/v2v/input_libvirt_vcenter_https.ml
@@ -36,9 +36,9 @@ let readahead_for_copying = Some (64 * 1024 * 1024)
 
 (* Subclass specialized for handling VMware vCenter over https. *)
 class input_libvirt_vcenter_https
-        input_password libvirt_uri parsed_uri server guest +        input_conn
input_password parsed_uri server guest  object
-  inherit input_libvirt input_password libvirt_uri guest
+  inherit input_libvirt input_conn input_password guest
 
   val saved_source_paths = Hashtbl.create 13
   val mutable dcPath = ""
@@ -65,8 +65,8 @@ object
      * that the domain is not running.  (RHBZ#1138586)
      *)
     let xml = Libvirt_utils.dumpxml ?password_file:input_password
-                                    ?conn:libvirt_uri guest in
-    let source, disks = parse_libvirt_xml ?conn:libvirt_uri xml in
+                                    ?conn:input_conn guest in
+    let source, disks = parse_libvirt_xml ?conn:input_conn xml in
 
     (* Find the <vmware:datacenterpath> element from the XML.  This
      * was added in libvirt >= 1.2.20.
diff --git a/v2v/input_libvirt_vddk.ml b/v2v/input_libvirt_vddk.ml
index d0633d958..630a07e26 100644
--- a/v2v/input_libvirt_vddk.ml
+++ b/v2v/input_libvirt_vddk.ml
@@ -95,7 +95,7 @@ let parse_input_options options    options
 
 (* Subclass specialized for handling VMware via nbdkit vddk plugin. *)
-class input_libvirt_vddk vddk_options input_password libvirt_uri parsed_uri
+class input_libvirt_vddk input_conn input_password vddk_options parsed_uri
                          guest    (* The VDDK path. *)
   let libdir @@ -200,7 +200,7 @@ See also \"INPUT FROM VDDK\" in the
virt-v2v(1) manual.") libNN
   in
 
 object
-  inherit input_libvirt input_password libvirt_uri guest as super
+  inherit input_libvirt input_conn input_password guest as super
 
   method precheck ()      error_unless_vddk_libdir ();
@@ -224,8 +224,8 @@ object
      * that the domain is not running.  (RHBZ#1138586)
      *)
     let xml = Libvirt_utils.dumpxml ?password_file:input_password
-                                    ?conn:libvirt_uri guest in
-    let source, disks = parse_libvirt_xml ?conn:libvirt_uri xml in
+                                    ?conn:input_conn guest in
+    let source, disks = parse_libvirt_xml ?conn:input_conn xml in
 
     (* Find the <vmware:moref> element from the XML.  This was added
      * in libvirt >= 3.7 and is required.
@@ -270,10 +270,10 @@ object
         match parsed_uri.Xml.uri_server with
         | Some server -> server
         | None ->
-           match libvirt_uri with
-           | Some libvirt_uri ->
+           match input_conn with
+           | Some input_conn ->
               error (f_"‘-ic %s’ URL does not contain a host name
field")
-                    libvirt_uri
+                    input_conn
            | None ->
               error (f_"you must use the ‘-ic’ parameter.  See
\"INPUT FROM VDDK\" in the virt-v2v(1) manual.") in
 
diff --git a/v2v/input_libvirt_vddk.mli b/v2v/input_libvirt_vddk.mli
index ef975c280..d321677d8 100644
--- a/v2v/input_libvirt_vddk.mli
+++ b/v2v/input_libvirt_vddk.mli
@@ -25,7 +25,7 @@ val print_input_options : unit -> unit
 val parse_input_options : (string * string) list -> vddk_options
 (** Print and parse vddk -io options. *)
 
-val input_libvirt_vddk : vddk_options -> string option -> string option
-> Xml.uri -> string -> Types.input
-(** [input_libvirt_vddk vddk_options input_password libvirt_uri parsed_uri
guest]
+val input_libvirt_vddk : string option -> string option -> vddk_options
-> Xml.uri -> string -> Types.input
+(** [input_libvirt_vddk input_conn input_password vddk_options parsed_uri
guest]
     creates and returns a {!Types.input} object specialized for reading
     the guest disks using the nbdkit vddk plugin. *)
diff --git a/v2v/input_libvirt_xen_ssh.ml b/v2v/input_libvirt_xen_ssh.ml
index 2e769cd0b..597957f92 100644
--- a/v2v/input_libvirt_xen_ssh.ml
+++ b/v2v/input_libvirt_xen_ssh.ml
@@ -30,9 +30,9 @@ open Input_libvirt_other
 open Printf
 
 (* Subclass specialized for handling Xen over SSH. *)
-class input_libvirt_xen_ssh input_password libvirt_uri parsed_uri server guest
+class input_libvirt_xen_ssh input_conn input_password parsed_uri server guest 
object
-  inherit input_libvirt input_password libvirt_uri guest
+  inherit input_libvirt input_conn input_password guest
 
   method precheck ()      if backend_is_libvirt () then
@@ -47,8 +47,8 @@ object
      * that the domain is not running.  (RHBZ#1138586)
      *)
     let xml = Libvirt_utils.dumpxml ?password_file:input_password
-                                    ?conn:libvirt_uri guest in
-    let source, disks = parse_libvirt_xml ?conn:libvirt_uri xml in
+                                    ?conn:input_conn guest in
+    let source, disks = parse_libvirt_xml ?conn:input_conn xml in
 
     (* Map the <source/> filename (which is relative to the remote
      * Xen server) to an ssh URI.  This is a JSON URI looking something
-- 
2.16.2

On Tuesday, 5 June 2018 11:52:22 CEST Richard W.M. Jones
wrote:
> Use -ip instead of --password-file, and various refactorings.
The series LGTM.
> It strikes me that we should probably deprecate and eventually remove
> virt-v2v-copy-to-local.  With the introduction of the new SSH and VDDK
> transports, and with RHEL 5 Xen becoming more irrelevant, it's no
> longer needed.
IMHO it is still useful, in case e.g.:
- the connection to the source is slow, so you want to fetch the guest
  locally at once
- you want to test various conversions of the same guest, so as above
  you fetch it once
- you want to test conversions offline (assuming the output is a local
  disk, or libvirt)

-- 
Pino Toscano