On 02/13/22 20:56, Nir Soffer wrote:
> Output modules can specify now request_size to override the default
> request size in nbdcopy.
> 
> The rhv-upload plugin is translating every NBD command to HTTP request,
> translated back to NBD command on imageio server. The HTTP client and
> server, and the NBD client on the imageio server side are synchronous
> and implemented in python, so they have high overhead per request. To
> get good performance we need to use larger request size.
> 
> Testing shows that request size of 4 MiB speeds up the copy disk phase
> from 14.7 seconds to 7.9 seconds (1.8x times faster). Request size of 8
> MiB is a little bit faster but is not compatible with VDDK input.
> 
> Here are stats extracted from imageio log when importing Fedora 35 image
> with 3 GiB of random data. For each copy, we have 4 connection stats.
> 
> Before:
> 
> connection 1 ops, 14.767843 s
> dispatch 4023 ops, 11.427662 s
> zero 38 ops, 0.053840 s, 327.91 MiB, 5.95 GiB/s
> write 3981 ops, 8.975877 s, 988.61 MiB, 110.14 MiB/s
> flush 4 ops, 0.001023 s
> 
> connection 1 ops, 14.770026 s
> dispatch 4006 ops, 11.408732 s
> zero 37 ops, 0.057205 s, 633.21 MiB, 10.81 GiB/s
> write 3965 ops, 8.907420 s, 986.65 MiB, 110.77 MiB/s
> flush 4 ops, 0.000280 s
> 
> connection 1 ops, 14.768180 s
> dispatch 4057 ops, 11.430712 s
> zero 42 ops, 0.030011 s, 470.47 MiB, 15.31 GiB/s
> write 4011 ops, 9.002055 s, 996.98 MiB, 110.75 MiB/s
> flush 4 ops, 0.000261 s
> 
> connection 1 ops, 14.770744 s
> dispatch 4037 ops, 11.462050 s
> zero 45 ops, 0.026668 s, 750.82 MiB, 27.49 GiB/s
> write 3988 ops, 9.002721 s, 989.36 MiB, 109.90 MiB/s
> flush 4 ops, 0.000282 s
> 
> After:
> 
> connection 1 ops, 7.940377 s
> dispatch 323 ops, 6.695582 s
> zero 37 ops, 0.079958 s, 641.12 MiB, 7.83 GiB/s
> write 282 ops, 6.300242 s, 1.01 GiB, 164.54 MiB/s
> flush 4 ops, 0.000537 s
> 
> connection 1 ops, 7.908156 s
> dispatch 305 ops, 6.643475 s
> zero 36 ops, 0.144166 s, 509.43 MiB, 3.45 GiB/s
> write 265 ops, 6.179187 s, 941.23 MiB, 152.32 MiB/s
> flush 4 ops, 0.000324 s
> 
> connection 1 ops, 7.942349 s
> dispatch 325 ops, 6.744800 s
> zero 45 ops, 0.185335 s, 622.19 MiB, 3.28 GiB/s
> write 276 ops, 6.236819 s, 995.45 MiB, 159.61 MiB/s
> flush 4 ops, 0.000369 s
> 
> connection 1 ops, 7.955572 s
> dispatch 317 ops, 6.721212 s
> zero 43 ops, 0.135771 s, 409.68 MiB, 2.95 GiB/s
> write 270 ops, 6.326366 s, 988.26 MiB, 156.21 MiB/s
> flush 4 ops, 0.001439 s
> ---
> 
> Changes since v1:
> 
> - Decrease request size to 4 MiB for compatibility with VDDK input.
>   (Richard)
> - Reimplement in a nicer way based on
>   
> https://github.com/libguestfs/virt-v2v/commit/08e764959ec9dadd71a95d22d3d88d647a18d165
>   (Richard)
> 
> v1 was here:
> https://listman.redhat.com/archives/libguestfs/2022-February/msg00183.html
> 
>  output/output.ml            |  1 +
>  output/output.mli           |  2 ++
>  output/output_disk.ml       |  2 ++
>  output/output_glance.ml     |  2 ++
>  output/output_json.ml       |  2 ++
>  output/output_libvirt.ml    |  2 ++
>  output/output_null.ml       |  2 ++
>  output/output_openstack.ml  |  2 +-
>  output/output_qemu.ml       |  2 ++
>  output/output_rhv.ml        |  2 ++
>  output/output_rhv_upload.ml |  7 +++++++
>  output/output_vdsm.ml       |  2 ++
>  v2v/v2v.ml                  | 11 ++++++++---
>  13 files changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/output/output.ml b/output/output.ml
> index 786ee5d5..7256b547 100644
> --- a/output/output.ml
> +++ b/output/output.ml
> @@ -39,20 +39,21 @@ type options = {
>  module type OUTPUT = sig
>    type poptions
>    type t
>    val to_string : options -> string
>    val query_output_options : unit -> unit
>    val parse_options : options -> Types.source -> poptions
>    val setup : string -> poptions -> Types.source -> t
>    val finalize : string -> poptions -> t ->
>                   Types.source -> Types.inspect -> Types.target_meta ->
>                   unit
> +  val request_size : int option
>  end
>  
>  let error_option_cannot_be_used_in_output_mode mode opt =
>    error (f_"-o %s: %s option cannot be used in this output mode") mode opt
>  
>  let get_disks dir =
>    let rec loop acc i =
>      let socket = sprintf "%s/in%d" dir i in
>      if Sys.file_exists socket then (
>        let size = Utils.with_nbd_connect_unix ~socket NBD.get_size in
> diff --git a/output/output.mli b/output/output.mli
> index eed204ed..8e3efd8e 100644
> --- a/output/output.mli
> +++ b/output/output.mli
> @@ -52,20 +52,22 @@ module type OUTPUT = sig
>  
>        Set up the output mode.  Sets up a disk pipeline
>        [dir // "outX"] for each output disk. *)
>  
>    val finalize : string -> poptions -> t ->
>                   Types.source -> Types.inspect -> Types.target_meta ->
>                   unit
>    (** [finalize dir poptions t inspect target_meta]
>  
>        Finalizes the conversion and writes metadata. *)
> +
> +  val request_size : int option
>  end
>  
>  (** Helper functions for output modes. *)
>  
>  val error_option_cannot_be_used_in_output_mode : string -> string -> unit
>  (** [error_option_cannot_be_used_in_output_mode mode option]
>      prints error message that option cannot be used in this output mode. *)
>  
>  val get_disks : string -> (int * int64) list
>  (** Examines the v2v directory and opens each input socket (in0 etc),
> diff --git a/output/output_disk.ml b/output/output_disk.ml
> index a05be559..bc5b4e1c 100644
> --- a/output/output_disk.ml
> +++ b/output/output_disk.ml
> @@ -105,11 +105,13 @@ module Disk = struct
>                  output_format output_name in
>  
>      let file = output_storage // output_name ^ ".xml" in
>      with_open_out file (fun chan -> DOM.doc_to_chan chan doc);
>  
>      if verbose () then (
>        eprintf "resulting local libvirt XML:\n";
>        DOM.doc_to_chan Stdlib.stderr doc;
>        eprintf "\n%!";
>      )
> +
> +  let request_size = None
>  end
> diff --git a/output/output_glance.ml b/output/output_glance.ml
> index 9ca406e8..e6a7f789 100644
> --- a/output/output_glance.ml
> +++ b/output/output_glance.ml
> @@ -131,11 +131,13 @@ module Glance = struct
>                      properties in
>          if run_command cmd <> 0 then
>            error (f_"glance: image upload to glance failed, see earlier 
> errors");
>  
>          (* Unlink the temporary files as soon as glance has got them. *)
>          try unlink disk with Unix_error _ -> ()
>      ) source.s_disks;
>  
>      (* Remove the temporary directory for the large files. *)
>      (try rmdir tmpdir with Unix_error _ -> ())
> +
> +  let request_size = None
>  end
> diff --git a/output/output_json.ml b/output/output_json.ml
> index 97883217..6e81b639 100644
> --- a/output/output_json.ml
> +++ b/output/output_json.ml
> @@ -141,11 +141,13 @@ module Json = struct
>        output_string Stdlib.stderr doc_string;
>        eprintf "\n\n%!";
>      );
>  
>      let file = output_storage // output_name ^ ".json" in
>      with_open_out file (
>        fun chan ->
>          output_string chan doc_string;
>          output_char chan '\n'
>      )
> +
> +  let request_size = None
>  end
> diff --git a/output/output_libvirt.ml b/output/output_libvirt.ml
> index 2236573f..e0d3432d 100644
> --- a/output/output_libvirt.ml
> +++ b/output/output_libvirt.ml
> @@ -210,11 +210,13 @@ module Libvirt_ = struct
>        warning (f_"the target hypervisor does not support a %s KVM guest") 
> arch;
>        []
>      ) else (
>        let node (* first matching <guest> *) = Xml.xpathobj_node obj 0 in
>        Xml.xpathctx_set_current_context xpathctx node;
>  
>        (* Get guest/features/* nodes. *)
>        let features = xpath_get_nodes xpathctx "features/*" in
>        List.map Xml.node_name features
>      )
> +
> +  let request_size = None
>  end
> diff --git a/output/output_null.ml b/output/output_null.ml
> index eeb15653..86d81eaa 100644
> --- a/output/output_null.ml
> +++ b/output/output_null.ml
> @@ -81,11 +81,13 @@ module Null = struct
>      List.iter (
>        fun (i, _) ->
>          if i > 0 then (
>            let output = sprintf "%s/out%d" dir i in
>            link socket output
>          )
>      ) disks
>  
>    let finalize dir () () source inspect target_meta =
>      () (* nothing to do *)
> +
> +  let request_size = None
>  end
> diff --git a/output/output_openstack.ml b/output/output_openstack.ml
> index 3d7a7882..d0af2ac7 100644
> --- a/output/output_openstack.ml
> +++ b/output/output_openstack.ml
> @@ -475,12 +475,12 @@ The os-* parameters and environment variables are 
> optional.
>      ) volume_ids
>  
>    (* UTC conversion time. *)
>    and iso_time =
>      let time = time () in
>      let tm = gmtime time in
>      sprintf "%04d/%02d/%02d %02d:%02d:%02d"
>        (tm.tm_year + 1900) (tm.tm_mon + 1) tm.tm_mday
>        tm.tm_hour tm.tm_min tm.tm_sec
>  
> -
> +  let request_size = None
>  end
> diff --git a/output/output_qemu.ml b/output/output_qemu.ml
> index 873a63b7..f8d2e171 100644
> --- a/output/output_qemu.ml
> +++ b/output/output_qemu.ml
> @@ -320,11 +320,13 @@ module QEMU = struct
>          Qemuopts.to_chan cmd chan
>      );
>  
>      Unix.chmod file 0o755;
>  
>      (* If -oo qemu-boot option was specified then we should boot the guest. 
> *)
>      if qemu_boot then (
>        let cmd = sprintf "%s &" (quote file) in
>        ignore (shell_command cmd)
>      )
> +
> +  let request_size = None
>  end
> diff --git a/output/output_rhv.ml b/output/output_rhv.ml
> index 022d96e3..119207fd 100644
> --- a/output/output_rhv.ml
> +++ b/output/output_rhv.ml
> @@ -275,11 +275,13 @@ module RHV = struct
>        Create_ovf.create_ovf source inspect target_meta sizes
>          output_alloc output_format output_name esd_uuid image_uuids vol_uuids
>          ~need_actual_sizes:true dir vm_uuid
>          Create_ovf.RHVExportStorageDomain in
>  
>      (* Write it to the metadata file. *)
>      let dir = esd_mp // esd_uuid // "master" // "vms" // vm_uuid in
>      Changeuid.mkdir changeuid_t dir 0o755;
>      let file = dir // vm_uuid ^ ".ovf" in
>      Changeuid.output changeuid_t file (fun chan -> DOM.doc_to_chan chan ovf)
> +
> +  let request_size = None
>  end
> diff --git a/output/output_rhv_upload.ml b/output/output_rhv_upload.ml
> index 49f13099..7c2434bd 100644
> --- a/output/output_rhv_upload.ml
> +++ b/output/output_rhv_upload.ml
> @@ -477,11 +477,18 @@ e command line has to match the number of guest disk 
> images (for this guest: %d)
>      let json_params =
>        match rhv_cluster_uuid with
>        | None -> assert false
>        | Some uuid -> ("rhv_cluster_uuid", JSON.String uuid) :: json_params in
>  
>      let ovf_file = dir // "vm.ovf" in
>      with_open_out ovf_file (fun chan -> output_string chan ovf);
>      if Python_script.run_command createvm_script json_params [ovf_file] <> 0
>      then
>        error (f_"failed to create virtual machine, see earlier errors")
> +
> +  (* The imageio server has high overhead per request. Using 4 MiB
> +   * request size is 1.8x times faster compared with nbdcopy default
> +   * request size (256k). Request size of 8 MiB is a little bit faster
> +   * but is is not compatible with VDDK input.
> +   *)
> +  let request_size = Some (4*1024*1024)
>  end
> diff --git a/output/output_vdsm.ml b/output/output_vdsm.ml
> index 14cbb961..a1e8c246 100644
> --- a/output/output_vdsm.ml
> +++ b/output/output_vdsm.ml
> @@ -218,11 +218,13 @@ For each disk you must supply one of each of these 
> options:
>                  output_alloc output_format output_name dd_uuid
>                  image_uuids
>                  vol_uuids
>                  dir
>                  vm_uuid
>                  ovf_flavour in
>  
>      (* Write it to the metadata file. *)
>      let file = ovf_output // vm_uuid ^ ".ovf" in
>      with_open_out file (fun chan -> DOM.doc_to_chan chan ovf)
> +
> +  let request_size = None
>  end
> diff --git a/v2v/v2v.ml b/v2v/v2v.ml
> index fddb0742..cadf864d 100644
> --- a/v2v/v2v.ml
> +++ b/v2v/v2v.ml
> @@ -583,32 +583,33 @@ read the man page virt-v2v(1).
>        List.rev acc
>    in
>    let disks = loop [] 0 in
>    let nr_disks = List.length disks in
>  
>    (* Copy the disks. *)
>    List.iter (
>      fun (i, input_socket, output_socket) ->
>        message (f_"Copying disk %d/%d") (i+1) nr_disks;
>  
> -      let input_uri = nbd_uri_of_socket input_socket
> +      let request_size = Output_module.request_size
> +      and input_uri = nbd_uri_of_socket input_socket
>        and output_uri = nbd_uri_of_socket output_socket in
>  
>        (* In verbose mode print some information about each
>         * side of the pipeline.
>         *)
>        if verbose () then (
>          nbdinfo ~content:true input_uri;
>          nbdinfo ~content:false output_uri
>        );
>  
> -      nbdcopy output_alloc input_uri output_uri
> +      nbdcopy ?request_size output_alloc input_uri output_uri
>    ) disks;
>  
>    (* End of copying phase. *)
>    unlink (tmpdir // "copy");
>  
>    (* Do the finalization step. *)
>    message (f_"Creating output metadata");
>    Output_module.finalize tmpdir output_poptions output_t
>      source inspect target_meta;
>  
> @@ -627,26 +628,30 @@ read the man page virt-v2v(1).
>   * appliance may be created there.  (RHBZ#1316479, RHBZ#2051394)
>   *)
>  and check_host_free_space () =
>    let free_space = StatVFS.free_space (StatVFS.statvfs large_tmpdir) in
>    debug "check_host_free_space: large_tmpdir=%s free_space=%Ld"
>          large_tmpdir free_space;
>    if free_space < 1_073_741_824L then
>      error (f_"insufficient free space in the conversion server temporary 
> directory %s (%s).\n\nEither free up space in that directory, or set the 
> LIBGUESTFS_CACHEDIR environment variable to point to another directory with 
> more than 1GB of free space.\n\nSee also the virt-v2v(1) manual, section 
> \"Minimum free space check in the host\".")
>            large_tmpdir (human_size free_space)
>  
> -and nbdcopy output_alloc input_uri output_uri =
> +and nbdcopy ?request_size output_alloc input_uri output_uri =
>    (* XXX It's possible that some output modes know whether
>     * --target-is-zero which would be a useful optimization.
>     *)
>    let cmd = ref [] in
>    List.push_back_list cmd [ "nbdcopy"; input_uri; output_uri ];
> +  (match request_size with
> +    | None -> ()
> +    | Some size -> List.push_back cmd (sprintf "--request-size=%d" size)
> +  );
>    List.push_back cmd "--flush";
>    (*List.push_back cmd "--verbose";*)
>    if not (quiet ()) then List.push_back cmd "--progress";
>    if output_alloc = Types.Preallocated then List.push_back cmd "--allocated";
>    let cmd = !cmd in
>  
>    if run_command cmd <> 0 then
>      error (f_"nbdcopy command failed, see earlier error messages")
>  
>  (* Run nbdinfo on a URI and dump the information to stderr.
> 

Acked-by: Laszlo Ersek <ler...@redhat.com>

_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Reply via email to