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

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(a)redhat.com&gt; --- 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