[Libguestfs] [PATCH v2v] Restore message about setting up the input and output
Laszlo Ersek
lersek at redhat.com
Wed Jan 19 15:01:13 UTC 2022
On 01/19/22 14:37, Richard W.M. Jones wrote:
> Old virt-v2v would print a summary of the input and output options
> before connecting to the input/output, looking something like this:
>
> [ 0.2] Opening the source -i libvirt -ic [etc]
>
> This gave reassurance that virt-v2v was doing something in the case
> where the source was slow or unreachable. In particular if you use
> -i libvirt with a vCenter URL, and the URL is wrong, libvirt hangs for
> a few minutes without printing anything.
>
> Modular virt-v2v rearranged things so the connecting phase was silent,
> which meant that in the case above virt-v2v appeared to hang for a few
> minutes printing nothing at all.
>
> This change adds to_string functions to all the input and output
> methods and uses them to print a message like:
>
> [ 0.0] Setting up the source: -i libvirt -ic [etc]
>
> Note the old "Opening the source" message now refers to the libguestfs
> handle open and connection to the NBD source disk pipeline. The hang
> still happens, but at least it's clearer what it's doing.
>
> Typical full output looks like this:
>
> $ virt-v2v -i disk /var/tmp/fedora-35.img -o disk -os /var/tmp/out
> [ 0.0] Setting up the source: -i disk /var/tmp/fedora-35.img
> [ 1.1] Opening the source
> [ 5.9] Inspecting the source
> [ 11.5] Checking for sufficient free disk space in the guest
> [ 11.5] Converting Fedora Linux 35 (Thirty Five) to run on KVM
> virt-v2v: warning: /files/boot/grub2/device.map/hd0 references unknown
> device "vda". You may have to fix this entry manually after conversion.
> virt-v2v: This guest has virtio drivers installed.
> [ 57.4] Mapping filesystem data to avoid copying unused and blank areas
> [ 61.0] Closing the overlay
> [ 61.7] Assigning disks to buses
> [ 61.7] Checking if the guest needs BIOS or UEFI to boot
> [ 61.7] Setting up the destination: -o disk -os /var/tmp/out
> [ 62.8] Copying disk 1/1
> █ 100% [****************************************]
> [ 81.7] Creating output metadata
> [ 81.7] Finishing off
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2041886
> Reported-by: Xiaodai Wang
> ---
> input/input.ml | 1 +
> input/input.mli | 4 ++++
> input/input_disk.ml | 2 ++
> input/input_libvirt.ml | 10 ++++++++++
> input/input_ova.ml | 2 ++
> input/input_vcenter_https.ml | 9 +++++++++
> input/input_vddk.ml | 9 +++++++++
> input/input_vmx.ml | 2 ++
> input/input_xen_ssh.ml | 9 +++++++++
> output/output.ml | 1 +
> output/output.mli | 4 ++++
> output/output_disk.ml | 6 ++++++
> output/output_glance.ml | 2 ++
> output/output_json.ml | 6 ++++++
> output/output_libvirt.ml | 6 ++++++
> output/output_null.ml | 2 ++
> output/output_openstack.ml | 2 ++
> output/output_qemu.ml | 6 ++++++
> output/output_rhv.ml | 2 ++
> output/output_rhv_upload.ml | 9 +++++++++
> output/output_vdsm.ml | 2 ++
> v2v/v2v.ml | 4 ++++
> 22 files changed, 100 insertions(+)
>
> diff --git a/input/input.ml b/input/input.ml
> index 00474becb1..b1175fa32f 100644
> --- a/input/input.ml
> +++ b/input/input.ml
> @@ -26,6 +26,7 @@ type options = {
> }
>
> module type INPUT = sig
> + val to_string : options -> string list -> string
> val setup : string -> options -> string list -> Types.source
> val query_input_options : unit -> unit
> end
> diff --git a/input/input.mli b/input/input.mli
> index 4f899b1d1c..b61df3e9fa 100644
> --- a/input/input.mli
> +++ b/input/input.mli
> @@ -26,6 +26,10 @@ type options = {
> }
>
> module type INPUT = sig
> + val to_string : options -> string list -> string
> + (** [to_string options args] converts the source to a printable
> + string (for messages). *)
> +
> val setup : string -> options -> string list -> Types.source
> (** [setup dir options args]
>
> diff --git a/input/input_disk.ml b/input/input_disk.ml
> index bcdaf78c55..2b21950a2a 100644
> --- a/input/input_disk.ml
> +++ b/input/input_disk.ml
> @@ -142,6 +142,8 @@ and detect_local_input_format { input_format } filenames =
> get_format formats
>
> module Disk = struct
> + let to_string options args = String.concat " " ("-i disk" :: args)
> +
> let setup dir options args =
> disk_source dir options args
>
> diff --git a/input/input_libvirt.ml b/input/input_libvirt.ml
> index f20082c2fa..33f61086ec 100644
> --- a/input/input_libvirt.ml
> +++ b/input/input_libvirt.ml
> @@ -129,6 +129,14 @@ and libvirt_xml_source _ args =
> source, disks
>
> module Libvirt_ = struct
> + let to_string options args =
> + let xs = "-i libvirt" :: args in
> + let xs =
> + match options.input_conn with
> + | Some ic -> ("-ic " ^ ic) :: xs
> + | None -> xs in
> + String.concat " " xs
> +
> let setup dir options args =
> let source, data = libvirt_source options args in
> libvirt_servers dir data;
> @@ -139,6 +147,8 @@ module Libvirt_ = struct
> end
>
> module LibvirtXML = struct
> + let to_string options args = String.concat " " ("-i libvirtxml" :: args)
> +
> let setup dir options args =
> let source, data = libvirt_xml_source options args in
> libvirt_servers dir data;
> diff --git a/input/input_ova.ml b/input/input_ova.ml
> index 0115f771cb..19c22d550e 100644
> --- a/input/input_ova.ml
> +++ b/input/input_ova.ml
> @@ -229,6 +229,8 @@ and error_missing_href href =
> error (f_"-i ova: OVF references file ‘%s’ which was not found in the OVA archive") href
>
> module OVA = struct
> + let to_string options args = String.concat " " ("-i ova" :: args)
> +
> let setup dir options args =
> ova_source dir options args
>
> diff --git a/input/input_vcenter_https.ml b/input/input_vcenter_https.ml
> index 24ac927dda..bcefed16db 100644
> --- a/input/input_vcenter_https.ml
> +++ b/input/input_vcenter_https.ml
> @@ -117,6 +117,15 @@ let rec vcenter_https_source dir options args =
> source
>
> module VCenterHTTPS = struct
> + let to_string options args =
> + let xs = args in
> + let xs =
> + match options.input_conn with
> + | Some ic -> ("-ic " ^ ic) :: xs
> + | None -> xs in
> + let xs = "-i libvirt" :: xs in
Huh, this was very non-intuitive. I thought "libvirt" here was an error,
but it's not.
> + String.concat " " xs
> +
> let setup dir options args =
> vcenter_https_source dir options args
>
> diff --git a/input/input_vddk.ml b/input/input_vddk.ml
> index 1cfb7f5ec3..b9a0b8bf5c 100644
> --- a/input/input_vddk.ml
> +++ b/input/input_vddk.ml
> @@ -193,6 +193,15 @@ and vddk_source dir options args =
> source
>
> module VDDK = struct
> + let to_string options args =
> + let xs = "-it vddk" :: args in
> + let xs =
> + match options.input_conn with
> + | Some ic -> ("-ic " ^ ic) :: xs
> + | None -> xs in
> + let xs = "-i libvirt" :: xs in
> + String.concat " " xs
> +
> let setup dir options args =
> vddk_source dir options args
Same here... I didn't realize the most frequently used transports for
accessing vmware were libvirt based...
>
> diff --git a/input/input_vmx.ml b/input/input_vmx.ml
> index 9065e8571d..6e8948f9d5 100644
> --- a/input/input_vmx.ml
> +++ b/input/input_vmx.ml
> @@ -118,6 +118,8 @@ and absolute_path_from_other_file other_filename filename =
> else (Filename.dirname (absolute_path other_filename)) // filename
>
> module VMX = struct
> + let to_string options args = String.concat " " ("-i vmx" :: args)
> +
> let setup dir options args =
> vmx_source dir options args
>
> diff --git a/input/input_xen_ssh.ml b/input/input_xen_ssh.ml
> index cb8b1f9124..f18ac5cf40 100644
> --- a/input/input_xen_ssh.ml
> +++ b/input/input_xen_ssh.ml
> @@ -112,6 +112,15 @@ let rec xen_ssh_source dir options args =
> source
>
> module XenSSH = struct
> + let to_string options args =
> + let xs = args in
> + let xs =
> + match options.input_conn with
> + | Some ic -> ("-ic " ^ ic) :: xs
> + | None -> xs in
> + let xs = "-i libvirt" :: xs in
> + String.concat " " xs
> +
> let setup dir options args =
> xen_ssh_source dir options args
>
> diff --git a/output/output.ml b/output/output.ml
> index 101da82a70..659d20ac31 100644
> --- a/output/output.ml
> +++ b/output/output.ml
> @@ -38,6 +38,7 @@ type options = {
>
> module type OUTPUT = sig
> type t
> + val to_string : options -> string
> val setup : string -> options -> Types.source -> t
> val finalize : string -> options ->
> Types.source -> Types.inspect -> Types.target_meta ->
> diff --git a/output/output.mli b/output/output.mli
> index 03d71daf67..ced2216106 100644
> --- a/output/output.mli
> +++ b/output/output.mli
> @@ -30,6 +30,10 @@ module type OUTPUT = sig
> type t
> (** Opaque data used by the output mode. *)
>
> + val to_string : options -> string
> + (** [to_string options] converts the destination to a printable
> + string (for messages). *)
> +
> val setup : string -> options -> Types.source -> t
> (** [setup dir options source]
>
> diff --git a/output/output_disk.ml b/output/output_disk.ml
> index eca3c727b7..386d031b46 100644
> --- a/output/output_disk.ml
> +++ b/output/output_disk.ml
> @@ -96,6 +96,12 @@ and disk_finalize dir source inspect target_meta
> module Disk = struct
> type t = unit
>
> + let to_string options =
> + "-o disk" ^
> + match options.output_storage with
> + | Some os -> " -os " ^ os
> + | None -> ""
> +
> let setup dir options source =
> if options.output_options <> [] then
> error (f_"no -oo (output options) are allowed here");
> diff --git a/output/output_glance.ml b/output/output_glance.ml
> index 0d7838dd38..85cbe58ea5 100644
> --- a/output/output_glance.ml
> +++ b/output/output_glance.ml
> @@ -122,6 +122,8 @@ and glance_finalize dir source inspect target_meta output_format tmpdir =
> module Glance = struct
> type t = string
>
> + let to_string options = "-o glance"
> +
> let setup dir options source =
> if options.output_options <> [] then
> error (f_"no -oo (output options) are allowed here");
> diff --git a/output/output_json.ml b/output/output_json.ml
> index bb0cdfeb5a..88fb4778d4 100644
> --- a/output/output_json.ml
> +++ b/output/output_json.ml
> @@ -134,6 +134,12 @@ and json_path os output_name json_disks_pattern i =
> module Json = struct
> type t = unit
>
> + let to_string options =
> + "-o json" ^
> + match options.output_storage with
> + | Some os -> " -os " ^ os
> + | None -> ""
> +
> let setup dir options source =
> let data = json_parse_options options in
> let output_name = get_output_name options source in
I tried to understand here whether "-oo json-disks-pattern" should be
logged here; however, I can't tell the status of that option. The top of
this file indicates the option is still supported, but the bottom of the
file states the opposite ("no -oo (output options) are allowed here").
> diff --git a/output/output_libvirt.ml b/output/output_libvirt.ml
> index 52c4540130..20333363b7 100644
> --- a/output/output_libvirt.ml
> +++ b/output/output_libvirt.ml
> @@ -198,6 +198,12 @@ and target_features_of_capabilities_doc doc arch =
> module Libvirt_ = struct
> type t = string * string
>
> + let to_string options =
> + "-o libvirt" ^
> + match options.output_storage with
> + | Some os -> " -os " ^ os
> + | None -> ""
> +
> let setup dir options source =
> if options.output_options <> [] then
> error (f_"no -oo (output options) are allowed here");
> diff --git a/output/output_null.ml b/output/output_null.ml
> index 34fbd6e148..56fb7ec63c 100644
> --- a/output/output_null.ml
> +++ b/output/output_null.ml
> @@ -76,6 +76,8 @@ and null_servers dir disks output_name =
> module Null = struct
> type t = unit
>
> + let to_string options = "-o null"
> +
> let setup dir options source =
> if options.output_options <> [] then
> error (f_"no -oo (output options) are allowed here");
> diff --git a/output/output_openstack.ml b/output/output_openstack.ml
> index 334a1fc2ae..1798548dc3 100644
> --- a/output/output_openstack.ml
> +++ b/output/output_openstack.ml
> @@ -462,6 +462,8 @@ and iso_time =
> module Openstack = struct
> type t = string list
>
> + let to_string options = "-o openstack"
> +
> let setup dir options source =
> let data = openstack_parse_options options in
> let output_name = get_output_name options source in
No need to print "-oo server-id=SERVER" and the other optional -oo options?
> diff --git a/output/output_qemu.ml b/output/output_qemu.ml
> index 0aac1eba2b..3d5d67820e 100644
> --- a/output/output_qemu.ml
> +++ b/output/output_qemu.ml
> @@ -315,6 +315,12 @@ and qemu_finalize dir source inspect target_meta
> module QEMU = struct
> type t = unit
>
> + let to_string options =
> + "-o qemu" ^
> + match options.output_storage with
> + | Some os -> " -os " ^ os
> + | None -> ""
> +
> let setup dir options source =
> let data = qemu_parse_options options in
> let output_name = get_output_name options source in
> diff --git a/output/output_rhv.ml b/output/output_rhv.ml
> index 6a67b7aa15..a386b9a58d 100644
> --- a/output/output_rhv.ml
> +++ b/output/output_rhv.ml
> @@ -266,6 +266,8 @@ and check_storage_domain domain_class os mp =
> module RHV = struct
> type t = string * string * string * string list * string list * int64 list
>
> + let to_string options = "-o rhv"
> +
> let setup dir options source =
> if options.output_options <> [] then
> error (f_"no -oo (output options) are allowed here");
Bunch of -oo options possible here too I think -- I'm missing the
"organizing principle" behind what options are printed and what are not.
> diff --git a/output/output_rhv_upload.ml b/output/output_rhv_upload.ml
> index 91e7be45bc..4d8dc1c135 100644
> --- a/output/output_rhv_upload.ml
> +++ b/output/output_rhv_upload.ml
> @@ -444,6 +444,15 @@ module RHVUpload = struct
> JSON.field list * string option * string option *
> string option * string
>
> + let to_string options =
> + "-o rhv-upload" ^
> + (match options.output_conn with
> + | Some oc -> " -oc " ^ oc
> + | None -> "") ^
> + (match options.output_storage with
> + | Some os -> " -os " ^ os
> + | None -> "")
> +
> let setup dir options source =
> let data = rhv_upload_parse_options options in
> let output_name = get_output_name options source in
> diff --git a/output/output_vdsm.ml b/output/output_vdsm.ml
> index ce0d5b5e4b..676ecf0010 100644
> --- a/output/output_vdsm.ml
> +++ b/output/output_vdsm.ml
> @@ -212,6 +212,8 @@ and vdsm_finalize dir source inspect target_meta
> module VDSM = struct
> type t = string * string * int64 list
>
> + let to_string options = "-o vdsm"
> +
> let setup dir options source =
> let data = vdsm_parse_options options in
> let output_name = get_output_name options source in
> diff --git a/v2v/v2v.ml b/v2v/v2v.ml
> index 47e6e9371a..d74cc21f0a 100644
> --- a/v2v/v2v.ml
> +++ b/v2v/v2v.ml
> @@ -532,6 +532,8 @@ read the man page virt-v2v(1).
> } in
>
> (* Start the input module (runs an NBD server in the background). *)
> + message (f_"Setting up the source: %s")
> + (Input_module.to_string input_options args);
> let source = Input_module.setup tmpdir input_options args in
>
> (* If --print-source then print the source metadata and exit. *)
> @@ -548,6 +550,8 @@ read the man page virt-v2v(1).
> unlink (tmpdir // "convert");
>
> (* Start the output module (runs an NBD server in the background). *)
> + message (f_"Setting up the destination: %s")
> + (Output_module.to_string output_options);
> let output_t = Output_module.setup tmpdir output_options source in
>
> (* Debug the v2vdir. *)
>
The code looks OK to me, but I'm not familiar enough with the various
input and output modules to really claim that the set of options printed
is -- or is not -- just right.
Reviewed-by: Laszlo Ersek <lersek at redhat.com>
Thanks
Laszlo
More information about the Libguestfs
mailing list