On 1/15/24 19:24, Richard W.M. Jones wrote:
 This is a straight refactor of the existing code that handles
ssh/scp
 into a new module.  In this commit I just copy the code around without
 doing any cleanup; cleanup will follow in subsequent commits.
 ---
  input/Makefile.am               |  6 ++-
  input/parse_domain_from_vmx.mli |  6 ---
  input/ssh.mli                   | 34 ++++++++++++++
  input/input_vmx.ml              |  8 ++--
  input/parse_domain_from_vmx.ml  | 60 ++-----------------------
  input/ssh.ml                    | 78 +++++++++++++++++++++++++++++++++
  6 files changed, 123 insertions(+), 69 deletions(-) 
"git show -W --no-renames --color-moved=zebra --color" works very well
on this patch.
One comment below:
 
 diff --git a/input/Makefile.am b/input/Makefile.am
 index de016d6076..4153f87885 100644
 --- a/input/Makefile.am
 +++ b/input/Makefile.am
 @@ -39,6 +39,7 @@ SOURCES_MLI = \
  	parse_domain_from_vmx.mli \
  	parse_libvirt_xml.mli \
  	parse_vmx.mli \
 +	ssh.mli \
  	vCenter.mli
  
  SOURCES_ML = \
 @@ -46,11 +47,12 @@ SOURCES_ML = \
  	parse_libvirt_xml.ml \
  	OVF.ml \
  	OVA.ml \
 -	parse_vmx.ml \
 -	parse_domain_from_vmx.ml \
  	nbdkit_curl.ml \
  	nbdkit_ssh.ml \
  	nbdkit_vddk.ml \
 +	ssh.ml \
 +	parse_vmx.ml \
 +	parse_domain_from_vmx.ml \
  	vCenter.ml \
  	input.ml \
  	input_disk.ml \
 diff --git a/input/parse_domain_from_vmx.mli b/input/parse_domain_from_vmx.mli
 index e354b32e44..42f8100ee7 100644
 --- a/input/parse_domain_from_vmx.mli
 +++ b/input/parse_domain_from_vmx.mli
 @@ -22,9 +22,3 @@ type vmx_source =
  
  val vmx_source_of_arg : [`SSH] option -> string -> vmx_source
  val parse_domain_from_vmx : vmx_source -> Types.source * string list
 -
 -(* XXX Exporting these is a hack. *)
 -val path_of_uri : Xml.uri -> string
 -val server_of_uri : Xml.uri -> string
 -val port_of_uri : Xml.uri -> int option
 -val remote_file_exists : Xml.uri -> string -> bool
 diff --git a/input/ssh.mli b/input/ssh.mli
 new file mode 100644
 index 0000000000..e9a1a6a8c7
 --- /dev/null
 +++ b/input/ssh.mli
 @@ -0,0 +1,34 @@
 +(* virt-v2v
 + * Copyright (C) 2009-2024 Red Hat Inc.
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License along
 + * with this program; if not, write to the Free Software Foundation, Inc.,
 + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 + *)
 +
 +(** Wrappers for finding and downloading remote files over ssh. *)
 +
 +val path_of_uri : Xml.uri -> string
 +val server_of_uri : Xml.uri -> string
 +val port_of_uri : Xml.uri -> int option
 +
 +(** [remote_file_exists ssh_uri path] checks that [path] exists
 +    on the remote server [ssh_uri] (note any path inside [ssh_uri]
 +    is ignored). *)
 +val remote_file_exists : Xml.uri -> string -> bool
 +
 +(** [scp_from_remote_to_temporary ssh_uri tmpdir filename]
 +    uses scp to copy the single remote file at [ssh_uri] to
 +    the local file called [tmpdir/filename].  It returns the
 +    final path [tmpdir/filename]. *)
 +val scp_from_remote_to_temporary : Xml.uri -> string -> string -> string
 diff --git a/input/input_vmx.ml b/input/input_vmx.ml
 index eed8a43356..bd20420ca0 100644
 --- a/input/input_vmx.ml
 +++ b/input/input_vmx.ml
 @@ -79,21 +79,21 @@ module VMX = struct
              let socket = sprintf "%s/in%d" dir i in
              On_exit.unlink socket;
  
 -            let vmx_path = path_of_uri uri in
 +            let vmx_path = Ssh.path_of_uri uri in
              let abs_path = absolute_path_from_other_file vmx_path filename in
              let flat_vmdk = PCRE.replace (PCRE.compile "\\.vmdk$")
                                "-flat.vmdk" abs_path in
  
              (* RHBZ#1774386 *)
 -            if not (remote_file_exists uri flat_vmdk) then
 +            if not (Ssh.remote_file_exists uri flat_vmdk) then
                error (f_"This transport does not support guests with snapshots. \
                          Either collapse the snapshots for this guest and try \
                          the conversion again, or use one of the alternate \
                          conversion methods described in \
                          virt-v2v-input-vmware(1) section \"NOTES\".");
  
 -            let server = server_of_uri uri in
 -            let port = Option.map string_of_int (port_of_uri uri) in
 +            let server = Ssh.server_of_uri uri in
 +            let port = Option.map string_of_int (Ssh.port_of_uri uri) in
              let user = uri.Xml.uri_user in
              let password =
                match options.input_password with
 diff --git a/input/parse_domain_from_vmx.ml b/input/parse_domain_from_vmx.ml
 index f24990f8aa..8cf5893c35 100644
 --- a/input/parse_domain_from_vmx.ml
 +++ b/input/parse_domain_from_vmx.ml
 @@ -51,61 +51,6 @@ let vmx_source_of_arg input_transport arg =
         error (f_"vmx URI path component looks incorrect");
       SSH uri
  
 -(* Return various fields from the URI.  The checks in vmx_source_of_arg
 - * should ensure that none of these assertions fail.
 - *)
 -let port_of_uri { Xml.uri_port } =
 -  match uri_port with i when i <= 0 -> None | i -> Some i
 -let server_of_uri { Xml.uri_server } =
 -  match uri_server with None -> assert false | Some s -> s
 -let path_of_uri { Xml.uri_path } =
 -  match uri_path with None -> assert false | Some p -> p
 -
 -(* 'scp' a remote file into a temporary local file, returning the path
 - * of the temporary local file.
 - *)
 -let scp_from_remote_to_temporary uri tmpdir filename =
 -  let localfile = tmpdir // filename in
 -
 -  let cmd =
 -    sprintf "scp%s%s %s%s:%s %s"
 -            (if verbose () then "" else " -q")
 -            (match port_of_uri uri with
 -             | None -> ""
 -             | Some port -> sprintf " -P %d" port)
 -            (match uri.Xml.uri_user with
 -             | None -> ""
 -             | Some user -> quote user ^ "@")
 -            (quote (server_of_uri uri))
 -            (quote (path_of_uri uri))
 -            (quote localfile) in
 -  if verbose () then
 -    eprintf "%s\n%!" cmd;
 -  if Sys.command cmd <> 0 then
 -    error (f_"could not copy the VMX file from the remote server, \
 -              see earlier error messages");
 -  localfile
 -
 -(* Test if [path] exists on the remote server. *)
 -let remote_file_exists uri path =
 -  let cmd =
 -    sprintf "ssh%s %s%s test -f %s"
 -            (match port_of_uri uri with
 -             | None -> ""
 -             | Some port -> sprintf " -p %d" port)
 -            (match uri.Xml.uri_user with
 -             | None -> ""
 -             | Some user -> quote user ^ "@")
 -            (quote (server_of_uri uri))
 -            (* Double quoting is necessary for 'ssh', first to protect
 -             * from the local shell, second to protect from the remote
 -             * shell. 
https://github.com/libguestfs/virt-v2v/issues/35#issuecomment-1741730963
 -             *)
 -            (quote (quote path)) in
 -  if verbose () then
 -    eprintf "%s\n%!" cmd;
 -  Sys.command cmd = 0
 -
  let rec find_disks vmx vmx_source =
    (* Set the s_disk_id field to an incrementing number. *)
    List.mapi
 @@ -390,7 +335,8 @@ let parse_domain_from_vmx vmx_source =
      match vmx_source with
      | File filename -> Parse_vmx.parse_file filename
      | SSH uri ->
 -       let filename = scp_from_remote_to_temporary uri tmpdir "source.vmx" in
 +       let filename = Ssh.scp_from_remote_to_temporary uri tmpdir
 +                        "source.vmx" in
         Parse_vmx.parse_file filename in
  
    let name =
 @@ -400,7 +346,7 @@ let parse_domain_from_vmx vmx_source =
         warning (f_"no displayName key found in VMX file");
         match vmx_source with
         | File filename -> name_from_disk filename
 -       | SSH uri -> name_from_disk (path_of_uri uri) in
 +       | SSH uri -> name_from_disk (Ssh.path_of_uri uri) in
  
    let genid =
      (* See: 
https://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg02019.html *)
 diff --git a/input/ssh.ml b/input/ssh.ml
 new file mode 100644
 index 0000000000..8e12899a8e
 --- /dev/null
 +++ b/input/ssh.ml
 @@ -0,0 +1,78 @@
 +(* virt-v2v
 + * Copyright (C) 2009-2020 Red Hat Inc. 
This is a new file; are you sure you don't want to update the (C)
notice's end year to 2024?
Reviewed-by: Laszlo Ersek <lersek(a)redhat.com>
Laszlo
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License along
 + * with this program; if not, write to the Free Software Foundation, Inc.,
 + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 + *)
 +
 +open Std_utils
 +open Tools_utils
 +open Common_gettext.Gettext
 +
 +open Printf
 +
 +(* Return various fields from the URI.  The checks in vmx_source_of_arg
 + * should ensure that none of these assertions fail.
 + *)
 +let port_of_uri { Xml.uri_port } =
 +  match uri_port with i when i <= 0 -> None | i -> Some i
 +let server_of_uri { Xml.uri_server } =
 +  match uri_server with None -> assert false | Some s -> s
 +let path_of_uri { Xml.uri_path } =
 +  match uri_path with None -> assert false | Some p -> p
 +
 +(* 'scp' a remote file into a temporary local file, returning the path
 + * of the temporary local file.
 + *)
 +let scp_from_remote_to_temporary uri tmpdir filename =
 +  let localfile = tmpdir // filename in
 +
 +  let cmd =
 +    sprintf "scp%s%s %s%s:%s %s"
 +            (if verbose () then "" else " -q")
 +            (match port_of_uri uri with
 +             | None -> ""
 +             | Some port -> sprintf " -P %d" port)
 +            (match uri.Xml.uri_user with
 +             | None -> ""
 +             | Some user -> quote user ^ "@")
 +            (quote (server_of_uri uri))
 +            (quote (path_of_uri uri))
 +            (quote localfile) in
 +  if verbose () then
 +    eprintf "%s\n%!" cmd;
 +  if Sys.command cmd <> 0 then
 +    error (f_"could not copy the VMX file from the remote server, \
 +              see earlier error messages");
 +  localfile
 +
 +(* Test if [path] exists on the remote server. *)
 +let remote_file_exists uri path =
 +  let cmd =
 +    sprintf "ssh%s %s%s test -f %s"
 +            (match port_of_uri uri with
 +             | None -> ""
 +             | Some port -> sprintf " -p %d" port)
 +            (match uri.Xml.uri_user with
 +             | None -> ""
 +             | Some user -> quote user ^ "@")
 +            (quote (server_of_uri uri))
 +            (* Double quoting is necessary for 'ssh', first to protect
 +             * from the local shell, second to protect from the remote
 +             * shell. 
https://github.com/libguestfs/virt-v2v/issues/35#issuecomment-1741730963
 +             *)
 +            (quote (quote path)) in
 +  if verbose () then
 +    eprintf "%s\n%!" cmd;
 +  Sys.command cmd = 0