[Libguestfs] [PATCH libnbd] copy: Always allow multiple connections

[Nir Soffer]

Using multiple connections when the server does not report
can_multi_conn is not safe in general, but for the special access
pattern in nbdcopy it is safe.

Use multiple connections are used even if the destination (e.g.
qemu-nbd) does not report the multi-conn flag.

Here is an example run with this change using qemu-nbd and qcow2
images. Using /dev/shm to get stable results.

When copying to memory, using 8 in-flight requests and request size of
128k best. I'm comparing also 2m requests to match qemu-img default io
size.

Testing with images on /dev/shm shows 25% speedup compared with single
connection. Testing with FC storage on real images shows 10% speedup[1].

$ qemu-img create -f qcow2 /dev/shm/dst.qcow2 6g
$ qemu-nbd -r -t -e 4 -f qcow2 -k /tmp/src.sock /var/tmp/fedora-32.qcow2 &
$ qemu-nbd -t -e 4 -f qcow2 -k /tmp/dst.sock /dev/shm/dst.qcow2 &
$ SRC=nbd+unix:///?socket=/tmp/src.sock
$ DST=nbd+unix:///?socket=/tmp/dst.sock

$ hyperfine -w3 -L s 131072,2097152 \
    ".libs/nbdcopy --requests=2 --request-size={s} --connections=4 --flush $SRC $DST" \
    ".libs/nbdcopy --requests=8 --request-size={s} --connections=1 --flush $SRC $DST" \
    "qemu-img convert -n -m 8 -W $SRC $DST"
Benchmark 1: .libs/nbdcopy --requests=2 --request-size=131072 --connections=4 --flush nbd+unix:///?socket=/tmp/src.sock nbd+unix:///?socket=/tmp/dst.sock
  Time (mean ± σ):     593.8 ms ±  16.6 ms    [User: 197.2 ms, System: 506.4 ms]
  Range (min … max):   567.9 ms … 617.8 ms    10 runs

Benchmark 2: .libs/nbdcopy --requests=8 --request-size=131072 --connections=1 --flush nbd+unix:///?socket=/tmp/src.sock nbd+unix:///?socket=/tmp/dst.sock
  Time (mean ± σ):     743.4 ms ±  82.4 ms    [User: 180.5 ms, System: 497.9 ms]
  Range (min … max):   631.2 ms … 816.5 ms    10 runs

Benchmark 3: qemu-img convert -n -m 8 -W nbd+unix:///?socket=/tmp/src.sock nbd+unix:///?socket=/tmp/dst.sock
  Time (mean ± σ):      1.029 s ±  0.022 s    [User: 165.7 ms, System: 655.2 ms]
  Range (min … max):    0.989 s …  1.067 s    10 runs

Benchmark 4: .libs/nbdcopy --requests=2 --request-size=2097152 --connections=4 --flush nbd+unix:///?socket=/tmp/src.sock nbd+unix:///?socket=/tmp/dst.sock
  Time (mean ± σ):      1.007 s ±  0.012 s    [User: 238.0 ms, System: 961.8 ms]
  Range (min … max):    0.992 s …  1.029 s    10 runs

Benchmark 5: .libs/nbdcopy --requests=8 --request-size=2097152 --connections=1 --flush nbd+unix:///?socket=/tmp/src.sock nbd+unix:///?socket=/tmp/dst.sock
  Time (mean ± σ):      1.129 s ±  0.013 s    [User: 184.4 ms, System: 743.6 ms]
  Range (min … max):    1.106 s …  1.149 s    10 runs

Summary
  '.libs/nbdcopy --requests=2 --request-size=131072 --connections=4 --flush nbd+unix:///?socket=/tmp/src.sock nbd+unix:///?socket=/tmp/dst.sock' ran
    1.25 ± 0.14 times faster than '.libs/nbdcopy --requests=8 --request-size=131072 --connections=1 --flush nbd+unix:///?socket=/tmp/src.sock nbd+unix:///?socket=/tmp/dst.sock'
    1.70 ± 0.05 times faster than '.libs/nbdcopy --requests=2 --request-size=2097152 --connections=4 --flush nbd+unix:///?socket=/tmp/src.sock nbd+unix:///?socket=/tmp/dst.sock'
    1.73 ± 0.06 times faster than 'qemu-img convert -n -m 8 -W nbd+unix:///?socket=/tmp/src.sock nbd+unix:///?socket=/tmp/dst.sock'
    1.90 ± 0.06 times faster than '.libs/nbdcopy --requests=8 --request-size=2097152 --connections=1 --flush nbd+unix:///?socket=/tmp/src.sock nbd+unix:///?socket=/tmp/dst.sock'

[1] https://listman.redhat.com/archives/libguestfs/2021-May/msg00124.html

Signed-off-by: Nir Soffer <nsoffer at redhat.com>
---
 copy/main.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/copy/main.c b/copy/main.c
index b9dbe1d..70a4e11 100644
--- a/copy/main.c
+++ b/copy/main.c
@@ -321,10 +321,6 @@ main (int argc, char *argv[])
     exit (EXIT_FAILURE);
   }
 
-  /* If multi-conn is not supported, force connections to 1. */
-  if (! src->ops->can_multi_conn (src) || ! dst->ops->can_multi_conn (dst))
-    connections = 1;
-
   /* Calculate the number of threads from the number of connections. */
   if (threads == 0) {
     long t;
@@ -379,15 +375,26 @@ main (int argc, char *argv[])
              synchronous ? "true" : "false");
   }
 
-  /* If multi-conn is enabled on either side, then at this point we
+  /* If using multiple connections, then at this point we
    * need to ask the backend to open the extra connections.
+   *
+   * Using multiple connections when the server does not report
+   * can_multi_conn is not safe in general, but is safe for the special
+   * access pattern in nbdcopy.
+   *
+   * - We split the image to 128m segments, and every segment is written
+   *   by single worker.
+   *
+   * - Writes do not overlap, and are never partial block that another
+   *   worker may modify at the same time.
+   *
+   * - We never read from the destination so we don't have caching
+   *   synchronization issue between clients.
    */
   if (connections > 1) {
     assert (threads == connections);
-    if (src->ops->can_multi_conn (src))
-      src->ops->start_multi_conn (src);
-    if (dst->ops->can_multi_conn (dst))
-      dst->ops->start_multi_conn (dst);
+    src->ops->start_multi_conn (src);
+    dst->ops->start_multi_conn (dst);
   }
 
   /* If the source is NBD and we couldn't negotiate meta
-- 
2.26.3