February 2022 - Libguestfs - libguestfs List Archives

[PATCH v2v] v2v/v2v.ml: Choose nbdcopy max requests for implicit buffer of 64M
by Richard W.M. Jones 15 Feb '22

15 Feb '22

Pick the nbdcopy --requests parameter to target an implicit buffer size of 64M inside nbdcopy. However don't set nbdcopy --request < 64. If request_size == 256K (the default) => requests = 256 If request_size == 8M => requests = 64 (buffer size 512M) --- v2v/v2v.ml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/v2v/v2v.ml b/v2v/v2v.ml index cadf864d5c..7bd47c1e7e 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -641,14 +641,27 @@ and nbdcopy ?request_size output_alloc input_uri output_uri = *) 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) ); + (* Choose max requests to target an implicit buffer size of 64M. *) + let requests = + let target_buffer_size = 64 * 1024 * 1024 in + let request_size = + match request_size with + | None -> 256 * 1024 (* default in nbdcopy 1.10+ *) + | Some size -> size in + min 64 (target_buffer_size / request_size) in + List.push_back cmd (sprintf "--requests=%d" requests); + 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 -- 2.35.1

2 3

[PATCH v2v] vddk: Use blocksize filter to split very large requests
by Richard W.M. Jones 15 Feb '22

15 Feb '22

Predictably this makes no difference to the current code since nbdcopy is using 256K requests. I will do some real testing when Nir pushes his change here: https://listman.redhat.com/archives/libguestfs/2022-February/msg00188.html Nir - do you want to actually increase that back to 8M? In theory (untested) it should be safe provided it's combined with something like this patch. Rich.

2 3

[PATCH v2] v2v/v2v.ml: Use larger request size for -o rhv-upload
by Nir Soffer 15 Feb '22

15 Feb '22

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/08e764959ec9dadd71a95d22d3d88… (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. -- 2.34.1

3 14

[PATCH] v2v/v2v.ml: Use larger request size for -o rhv-upload
by Nir Soffer 14 Feb '22

14 Feb '22

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 8MiB is best, speeding up the copy disk phase from 14.7 seconds to 7.7 seconds (1.9x times faster). 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.776159 s dispatch 181 ops, 6.701100 s zero 27 ops, 0.219959 s, 5.97 MiB, 27.15 MiB/s write 150 ops, 6.266066 s, 983.13 MiB, 156.90 MiB/s flush 4 ops, 0.000299 s connection 1 ops, 7.805616 s dispatch 187 ops, 6.643718 s zero 30 ops, 0.227808 s, 809.01 MiB, 3.47 GiB/s write 153 ops, 6.306260 s, 1.02 GiB, 165.81 MiB/s flush 4 ops, 0.000306 s connection 1 ops, 7.780301 s dispatch 191 ops, 6.535249 s zero 47 ops, 0.228495 s, 693.31 MiB, 2.96 GiB/s write 140 ops, 6.033484 s, 958.23 MiB, 158.82 MiB/s flush 4 ops, 0.001618 s connection 1 ops, 7.829294 s dispatch 213 ops, 6.594207 s zero 56 ops, 0.297876 s, 674.12 MiB, 2.21 GiB/s write 153 ops, 6.070786 s, 974.56 MiB, 160.53 MiB/s flush 4 ops, 0.000318 s This is an ugly hack; the preferred request size should be a function of the output module that only output_rhv_upload will override, but I don't know how to implement this with the current code. Another way is to add this as an output option; this will make it easier to test and find the best setting that works in a real environment, or tweak the value in a specific environment if needed. --- v2v/v2v.ml | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/v2v/v2v.ml b/v2v/v2v.ml index fddb0742..b21e2737 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -578,37 +578,45 @@ read the man page virt-v2v(1). let input_socket = sprintf "%s/in%d" tmpdir i and output_socket = sprintf "%s/out%d" tmpdir i in if Sys.file_exists input_socket && Sys.file_exists output_socket then loop ((i, input_socket, output_socket) :: acc) (i+1) else List.rev acc in let disks = loop [] 0 in let nr_disks = List.length disks in + (* XXX This is a hack for -o rhv-upload that works best with larger + * request size. + *) + let request_size = + match output_mode with + | `RHV_Upload -> 8*1024*1024 + | _ -> 0 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 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 output_alloc input_uri output_uri request_size ) 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 +635,28 @@ 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 output_alloc input_uri output_uri request_size = (* 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 ]; + if request_size != 0 then + List.push_back_list cmd ["--request-size"; string_of_int request_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. -- 2.34.1

2 5

[nbdkit PATCH] RFC: noextents: Add extentmode=data config parameter
by Eric Blake 12 Feb '22

12 Feb '22

While writing tests for an nbdcopy bug, I found myself wanting a way to easily view an entire image as data, but without disabling extents support altogether. The existing extentlist filter can do this, but requires a secondary file. Still an RFC because I need testsuite coverage similar to test-nozero.sh (eek - we don't have any direct test of the noextent filter, but only indirect coverage through other tests). Also, are there any other extentmode=MODE values that might make sense? --- filters/noextents/nbdkit-noextents-filter.pod | 32 ++++++++++-- filters/noextents/noextents.c | 50 ++++++++++++++++++- 2 files changed, 75 insertions(+), 7 deletions(-) diff --git a/filters/noextents/nbdkit-noextents-filter.pod b/filters/noextents/nbdkit-noextents-filter.pod index 891b197d..aac2f097 100644 --- a/filters/noextents/nbdkit-noextents-filter.pod +++ b/filters/noextents/nbdkit-noextents-filter.pod @@ -4,7 +4,7 @@ nbdkit-noextents-filter - disable extents in the underlying plugin =head1 SYNOPSIS - nbdkit --filter=noextents plugin + nbdkit --filter=noextents plugin [plugin-args...] [extentmode=MODE] =head1 DESCRIPTION @@ -23,9 +23,31 @@ performance (C<tmpfs> is known to be one such system). =head1 PARAMETERS -There are no parameters specific to nbdkit-noextents-filter. Any -parameters are passed through to and processed by the underlying -plugin in the normal way. +The parameter C<extentmode> is optional, and controls which mode the +filter will use. + +=over 4 + +=item B<extentmode=mask> + +Extent support is not advertised to the client; clients should not +query for extent information, and must assume the entire disk is +allocated. + +This is the default if the C<extentmode> parameter is not specified. + +=item B<extentmode=data> + +(nbdkit E<ge> 1.30) + +Extent support is advertised, but extent requests from the client will +be answered with a claim that the entire disk forms a single allocated +data extent. + +=back + +All other parameters are passed through to and processed by the +underlying plugin in the normal way. =head1 FILES @@ -61,4 +83,4 @@ Richard W.M. Jones =head1 COPYRIGHT -Copyright (C) 2019 Red Hat Inc. +Copyright (C) 2019-2022 Red Hat Inc. diff --git a/filters/noextents/noextents.c b/filters/noextents/noextents.c index f3044809..36231a35 100644 --- a/filters/noextents/noextents.c +++ b/filters/noextents/noextents.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2019 Red Hat Inc. + * Copyright (C) 2019-2022 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -32,19 +32,65 @@ #include <config.h> +#include <string.h> +#include <assert.h> + #include <nbdkit-filter.h> +static enum ExtentMode { + MASK, + DATA, +} extentmode; + +static int +noextents_config (nbdkit_next_config *next, nbdkit_backend *nxdata, + const char *key, const char *value) +{ + if (strcmp (key, "extentmode") == 0) { + if (strcmp (value, "mask") == 0) + extentmode = MASK; + else if (strcmp (value, "data") == 0) + extentmode = DATA; + else { + nbdkit_error ("unknown extentmode '%s'", value); + return -1; + } + return 0; + } + + return next (nxdata, key, value); +} + +#define noextents_config_help \ + "extentmode=<MODE> One of 'mask' (default), 'data'.\n" + +/* Advertise desired extents support. */ static int noextents_can_extents (nbdkit_next *next, void *handle) { - return 0; + return extentmode == DATA; +} + +/* Produce single data extent. */ +static int +noextents_extents (nbdkit_next *next, + void *handle, uint32_t count, uint64_t offset, + uint32_t flags, + struct nbdkit_extents *ret_extents, + int *err) +{ + assert (extentmode == DATA); + return nbdkit_add_extent (ret_extents, offset, count, 0); } static struct nbdkit_filter filter = { .name = "noextents", .longname = "nbdkit noextents filter", + .config = noextents_config, + .config_help = noextents_config_help, .can_extents = noextents_can_extents, + .extents = noextents_extents, }; NBDKIT_REGISTER_FILTER(filter) -- 2.34.1

3 5

[nbdkit PATCH] nbd: Opt in to libnbd pread_initialize speedup
by Eric Blake 11 Feb '22

11 Feb '22

Our nbd plugin has always properly checked for asynch errors (and thus has never been at risk of a vulnerability similar to CVE-2022-0485 just fixed in nbdcopy). What's more, the nbdkit core guarantees (since commit b1ce255e in 2019, v1.13.1) that the buffer handed to a .pread callback has been pre-sanitized to not leak heap contents, regardless of how buggy a plugin might be (see server/protocol.c:protocol_recv_request_send_reply(), which uses server/threadlocal.c:threadlocal_buffer() for a safe buffer). Thus, we do not need libnbd to spend time re-initializing the buffer, if libnbd is new enough to give us that knob. --- I know that Laszlo was sceptical whether any real client might want to use the libnbd knob [1], but I'm actually comfortable with this one. [1] https://listman.redhat.com/archives/libguestfs/2022-February/msg00144.html plugins/nbd/nbd.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c index ae595ea7..2e154e8d 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2017-2020, 2022 Red Hat Inc. + * Copyright (C) 2017-2022 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -637,6 +637,15 @@ nbdplug_open_handle (int readonly, const char *client_export) #if LIBNBD_HAVE_NBD_SET_FULL_INFO if (nbd_set_full_info (h->nbd, 1) == -1) goto errnbd; +#endif +#if LIBNBD_HAVE_NBD_SET_PREAD_INITIALIZE + /* nbdkit guarantees that the buffers passed to our .pread callback + * are pre-initialized; and we in turn ensure that the buffer is not + * dereferenced if the NBD server replied with an error. Thus, we + * are safe opting in to this libnbd speedup. + */ + if (nbd_set_pread_initialize (h->nbd, false) == -1) + goto errnbd; #endif if (dynamic_export && uri) { #if LIBNBD_HAVE_NBD_SET_OPT_MODE -- 2.34.1

2 3

working more easily on libguestfs-common submodule updates
by Laszlo Ersek 11 Feb '22

11 Feb '22

Hi, sorry if this is common knowledge, but I figured I'd share it. I have the following clones: /home/lacos/src/v2v/guestfs-tools /home/lacos/src/v2v/libguestfs /home/lacos/src/v2v/libguestfs-common /home/lacos/src/v2v/virt-v2v In order to modify "libguestfs-common", and immediately put the changes to use in the other three projects (the superprojects); i.e. libguestfs, guestfs-tools, and virt-v2v: (1) Create an *identically-named* development branch in all four projects. (2) In libguestfs-common, implement the change(s) on the new branch (commit them). (3) In the superprojects, run git submodule update --force --init git submodule sync (4) In the three superprojects, apply a patch like this (on the new branch): diff --git a/.gitmodules b/.gitmodules index 1343142128f6..7b1a84418c2b 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,4 @@ [submodule "common"] path = common - url = https://github.com/libguestfs/libguestfs-common + url = file:///home/lacos/src/v2v/libguestfs-common + branch = . (Commit it.) (5) In the superprojects, run git submodule sync git submodule update --force --remote (Commit the result.) (6) Every time a new change is committed in the libguestfs-common clone, re-run the "git submodule update --remote --force" in the superprojects, and commit the results. (7) After returning to the master branch in the superprojects, re-run step (3). Thanks Laszlo

1 0

[PATCH libnbd 0/9] golang: Safer, easier to use, and faster AioBuffer
by Nir Soffer 10 Feb '22

10 Feb '22

Improve AioBuffer to make it safer, easier to use, and faster when integrating with other Go APIs. New Go specific APIs: - MakeAioBufferZero() - creates a new buffer using calloc(), to make it easy and efficient to use a zeroed buffer. - AioBuffer.Slice() - create a slice backed by the underlying buffer without copying the contents of the buffer. Performance improments: - FromBytes() is 3 time faster - Code using Bytes() should use Slice() now. aio_copy example shows up to 260% speedup. Improve testing: - New AioBuffer tests - New AioBuffer benchmarks Documention: - AioBuffer is fully documnted now. Nir Soffer (9): golang: tests: Add test for AioBuffer golang: aio_buffer.go: Make it safer to use golang: aio_buffer.go: Add missing documentation golang: aio_buffer.go: Add MakeAioBufferZero() golang: aio_buffer.go: Add Slice() golang: tests: Use AioBuffer.Slice() golang: aio_buffer.go: Speed up FromBytes() golang: aio_buffer.go: Benchmark copy flows golang: examples: aio_copy: Simplify using AioBuffer golang/Makefile.am | 1 + golang/aio_buffer.go | 39 ++++- golang/examples/aio_copy/aio_copy.go | 29 +--- golang/libnbd_500_aio_pread_test.go | 2 +- golang/libnbd_510_aio_pwrite_test.go | 8 +- golang/libnbd_620_aio_buffer_test.go | 236 +++++++++++++++++++++++++++ 6 files changed, 281 insertions(+), 34 deletions(-) create mode 100644 golang/libnbd_620_aio_buffer_test.go -- 2.34.1

3 25

[PATCH libnbd] generator/Go.ml: Simplify copy_uint32_array
by Nir Soffer 10 Feb '22

10 Feb '22

Create a slice backed up by the entries pointer, and copy the data with builtin copy(). This can be 3x times faster but I did not measure it. Eric posted a similar patch[1] last year, but the patch seems to be stuck with unrelated incomplete work. [1] https://listman.redhat.com/archives/libguestfs/2021-December/msg00065.html Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- generator/GoLang.ml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/generator/GoLang.ml b/generator/GoLang.ml index eb3aa263..73838199 100644 --- a/generator/GoLang.ml +++ b/generator/GoLang.ml @@ -1,13 +1,13 @@ (* hey emacs, this is OCaml code: -*- tuareg -*- *) (* nbd client library in userspace: generator - * Copyright (C) 2013-2020 Red Hat Inc. + * Copyright (C) 2013-2022 Red Hat Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public * License as published by the Free Software Foundation; either * version 2 of the License, or (at your option) any later version. * * This library 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 * Lesser General Public License for more details. @@ -508,24 +508,24 @@ package libnbd #include \"wrappers.h\" */ import \"C\" import \"unsafe\" /* Closures. */ func copy_uint32_array (entries *C.uint32_t, count C.size_t) []uint32 { ret := make([]uint32, int (count)) - for i := 0; i < int (count); i++ { - entry := (*C.uint32_t) (unsafe.Pointer(uintptr(unsafe.Pointer(entries)) + (unsafe.Sizeof(*entries) * uintptr(i)))) - ret[i] = uint32 (*entry) - } + // See https://github.com/golang/go/wiki/cgo#turning-c-arrays-into-go-slices + // TODO: Use unsafe.Slice() when we require Go 1.17. + s := (*[1<<30]uint32)(unsafe.Pointer(entries))[:count:count] + copy(ret, s) return ret } "; List.iter ( fun { cbname; cbargs } -> let uname = camel_case cbname in pr "type %sCallback func (" uname; let comma = ref false in List.iter ( -- 2.34.1

3 3

[PATCH 0/5] Fix rhv-upload output
by Nir Soffer 10 Feb '22

10 Feb '22

Fix problems in new rhv-upload implementation: - The plugin does not flush to all connections in flush() - The plugin does not close all connections in cleanup() - Idle connections are closed in imageio server, and we don't have a safe way to recover. - virt-v2v try to get disk allocation using imageio output, but imageio output does not support extents. Even if imageio output will support extents, the call is done after the transfer was finalized so it does not have access to storage. Problems not fixed yet: - Image transfer is finalized *before* closing the connection to imageio - this will always time out with RHV < 4.4.9, and succeeds by mistake with RHV 4.4.9 due to a regression that will be fixed in 4.4.10. This will be a non-issue in next RHV version[1]. To support older RHV versions, virt-v2v must finalize the image transfer *after* closing the output. Tested on RHEL 8.6 with upstream nbdkit and libnbd. [1] https://github.com/oVirt/ovirt-imageio/pull/15 Fixes https://bugzilla.redhat.com/2032324 Nir Soffer (5): output/rhv-upload-plugin: Fix flush and close v2v/lib/util.ml: Get disk allocation from input output/rhv-upload-plugin: Extract send_flush() helper output/rhv-upload-plugin: Track http last request time output/rhv-upload-plugin: Keep connections alive lib/utils.ml | 2 +- output/rhv-upload-plugin.py | 151 ++++++++++++++++++++++++++---------- 2 files changed, 113 insertions(+), 40 deletions(-) -- 2.33.1

3 30