Re: [Libguestfs] [PATCH libnbd 3/3] copy: Do not initialize read buffer
On Sun, Mar 06, 2022 at 10:27:30PM +0200, Nir Soffer wrote:
nbdcopy checks pread error now, so we will never leak uninitialized
data
from the heap to the destination server. Testing show 3-8% speedup when
copying a real image.
+++ b/copy/nbd-ops.c
@@ -52,20 +52,21 @@ static void
open_one_nbd_handle (struct rw_nbd *rwn)
{
struct nbd_handle *nbd;
nbd = nbd_create ();
if (nbd == NULL) {
fprintf (stderr, "%s: %s\n", prog, nbd_get_error ());
exit (EXIT_FAILURE);
}
+ nbd_set_pread_initialize (nbd, false);
nbd_set_debug (nbd, verbose);
Pre-existing that we did not check for failure from nbd_set_debug(),
so it is not made worse by not checking for failure of
nbd_set_pread_initialize().
Then again, nbd_set_debug() is currently documented as being able to
fail, but in practice cannot - we do not restrict it to a subset of
states, and its implementation is dirt-simple in lib/debug.c. We may
want (as a separate patch) to tweak this function to be mared as
may_set_error=false, the way nbd_get_debug() is (as long as such
change does not impact the API).
Similarly, nbd_set_pread_initialize() has no restrictions on which
states it can be used in, so maybe we should also mark it as
may_set_error=false. Contrast that with things like
nbd_set_request_block_size(), which really do make sense to limit to
certain states (once negotiation is done, changing the flag has no
effect).
So we may have further cleanups to do, but once you add the comments
requested by Rich throughout the series, and the error checking I
suggested in 2/3, with the series.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org