[Libguestfs] Re: [PATCH virt-v2v 4/6] virt-v2v: -i vmx: Remove dependency of ssh.ml on Xml.uri

Xml.uri is a convenient way to pass the multiple ssh fields to virt-v2v and is still used internally by the -i vmx code. However don't leak this awkward implementation into the new ssh.ml module. Like Nbdkit_ssh, we'll only deal with the explicit (port, server, user, path) fields separately. Note after this refactoring: - The new Ssh module interface looks broadly similar to the Nbdkit_ssh interface. - vmx_source_of_arg assertions are no longer required inside the new Ssh module, which was a mild layering violation before. --- input/ssh.mli | 21 ++++++++++----------- input/input_vmx.ml | 21 ++++++++++++++++----- input/parse_domain_from_vmx.ml | 20 ++++++++++++++++++-- input/ssh.ml | 32 +++++++++++--------------------- 4 files changed, 55 insertions(+), 39 deletions(-)

diff --git a/input/ssh.mli b/input/ssh.mli index 62e78bd3e8..1f39cd1bea 100644 --- a/input/ssh.mli +++ b/input/ssh.mli @@ -18,16 +18,15 @@ (** 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 password ?port server ?user path] + checks that [path] exists on the remote server. *) +val remote_file_exists : password:Nbdkit_ssh.password -> + ?port:string -> server:string -> ?user:string -> + string -> bool -(** [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 - -(** [download_file ssh_uri output] - uses scp to copy the single remote file at [ssh_uri] to +(** [download_file password ?port server ?user path output] + uses scp to copy the single remote file at [path] to the local file called [output]. *) -val download_file : Xml.uri -> string -> unit +val download_file : password:Nbdkit_ssh.password -> + ?port:string -> server:string -> ?user:string -> string -> + string -> unit

diff --git a/input/ssh.ml b/input/ssh.ml index fbc0e54f5d..4e0e420e20 100644 --- a/input/ssh.ml +++ b/input/ssh.ml @@ -22,29 +22,19 @@ 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 local file. *)

-let download_file uri output = +let download_file ~password ?port ~server ?user path output =

let cmd = sprintf "scp%s%s %s%s:%s %s" (if verbose () then "" else " -q") - (match port_of_uri uri with + (match port with | None -> "" - | Some port -> sprintf " -P %d" port) - (match uri.Xml.uri_user with + | Some port -> sprintf " -P %s" port) + (match user with | None -> "" | Some user -> quote user ^ "@") - (quote (server_of_uri uri)) - (quote (path_of_uri uri)) + (quote server) + (quote path) (quote output) in if verbose () then eprintf "%s\n%!" cmd;

@@ -53,16 +43,16 @@ let download_file uri output = see earlier error messages") (* Test if [path] exists on the remote server. *) -let remote_file_exists uri path = +let remote_file_exists ~password ?port ~server ?user path = let cmd = sprintf "ssh%s %s%s test -f %s" - (match port_of_uri uri with + (match port with | None -> "" - | Some port -> sprintf " -p %d" port) - (match uri.Xml.uri_user with + | Some port -> sprintf " -p %s" port) + (match user with | None -> "" | Some user -> quote user ^ "@") - (quote (server_of_uri uri)) + (quote server) (* 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

diff --git a/input/input_vmx.ml b/input/input_vmx.ml index b9cce10fed..b3426fa242 100644 --- a/input/input_vmx.ml +++ b/input/input_vmx.ml @@ -83,22 +83,33 @@ module VMX = struct let socket = sprintf "%s/in%d" dir i in On_exit.unlink socket; - let vmx_path = Ssh.path_of_uri uri in + let vmx_path = + match uri.uri_path with + | None -> assert false (* checked by vmx_source_of_arg *) + | Some path -> path 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 + let server = + match uri.uri_server with + | None -> assert false (* checked by vmx_source_of_arg *) + | Some server -> server in + let port = + match uri.uri_port with + | i when i <= 0 -> None + | i -> Some (string_of_int i) in + let user = uri.Xml.uri_user in + (* RHBZ#1774386 *) - if not (Ssh.remote_file_exists uri flat_vmdk) then + if not (Ssh.remote_file_exists ~password ?port ~server ?user + 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 = 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 cor = dir // "convert" in let bandwidth = options.bandwidth in let nbdkit = Nbdkit_ssh.create_ssh ?bandwidth ~cor ~password

diff --git a/input/parse_domain_from_vmx.ml b/input/parse_domain_from_vmx.ml index 94ae9957e3..99c86b1ae0 100644 --- a/input/parse_domain_from_vmx.ml +++ b/input/parse_domain_from_vmx.ml @@ -335,8 +335,21 @@ let parse_domain_from_vmx vmx_source = match vmx_source with | File filename -> Parse_vmx.parse_file filename | SSH (password, uri) ->

+ let server = + match uri.uri_server with + | None -> assert false (* checked by vmx_source_of_arg *) + | Some server -> server in + let port = + match uri.uri_port with + | i when i <= 0 -> None + | i -> Some (string_of_int i) in + let user = uri.Xml.uri_user in + let path = + match uri.uri_path with + | None -> assert false (* checked by vmx_source_of_arg *) + | Some path -> path in let filename = tmpdir // "source.vmx" in - Ssh.download_file uri filename; + Ssh.download_file ~password ?port ~server ?user path filename; Parse_vmx.parse_file filename in let name =

@@ -346,7 +359,10 @@ 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 (Ssh.path_of_uri uri) in + | SSH (_, uri) -> + match uri.uri_path with + | None -> assert false (* checked by vmx_source_of_arg *) + | Some path -> name_from_disk path in let genid = (* See: https://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg02019.html *)