nbd: Advertise multi-conn for shared read-only connections

The NBD specification defines NBD_FLAG_CAN_MULTI_CONN, which can be
advertised when the server promises cache consistency between
simultaneous clients (basically, rules that determine what FUA and
flush from one client are able to guarantee for reads from another
client).  When we don't permit simultaneous clients (such as qemu-nbd
without -e), the bit makes no sense; and for writable images, we
probably have a lot more work before we can declare that actions from
one client are cache-consistent with actions from another.  But for
read-only images, where flush isn't changing any data, we might as
well advertise multi-conn support.  What's more, advertisement of the
bit makes it easier for clients to determine if 'qemu-nbd -e' was in
use, where a second connection will succeed rather than hang until the
first client goes away.

This patch affects qemu as server in advertising the bit.  We may want
to consider patches to qemu as client to attempt parallel connections
for higher throughput by spreading the load over those connections
when a server advertises multi-conn, but for now sticking to one
connection per nbd:// BDS is okay.

See also: https://bugzilla.redhat.com/1708300
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190815185024.7010-1-eblake@redhat.com>
[eblake: tweak blockdev-nbd.c to not request shared when writable,
fix iotest 233]
Reviewed-by: John Snow <jsnow@redhat.com>

diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index c621686131fd86cae73d24dec30afb43f5e7d32a..1fcfdb0997c6441708dd7391128f780be6580c9d 100644 (file)
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -188,7 +188,7 @@ void qmp_nbd_server_add(const char *device, bool has_name, const char *name,
     }
 
     exp = nbd_export_new(bs, 0, len, name, NULL, bitmap,
-                         writable ? 0 : NBD_FLAG_READ_ONLY,
+                         writable ? 0 : NBD_FLAG_READ_ONLY, !writable,
                          NULL, false, on_eject_blk, errp);
     if (!exp) {
         return;

diff --git a/docs/interop/nbd.txt b/docs/interop/nbd.txt
index fc64473e02b2b570c729a907e35c7296763dcbb1..6dfec7f476476702f4bbaeaa8558d786d8314563 100644 (file)
--- a/docs/interop/nbd.txt
+++ b/docs/interop/nbd.txt
@@ -53,3 +53,4 @@ the operation of that feature.
 * 2.12: NBD_CMD_BLOCK_STATUS for "base:allocation"
 * 3.0: NBD_OPT_STARTTLS with TLS Pre-Shared Keys (PSK),
 NBD_CMD_BLOCK_STATUS for "qemu:dirty-bitmap:", NBD_CMD_CACHE
+* 4.2: NBD_FLAG_CAN_MULTI_CONN for sharable read-only exports

diff --git a/include/block/nbd.h b/include/block/nbd.h
index 7b36d672f0468b7ef334ae77baf7c6072661cf6c..991fd52a5134b77ac8d75cd87d551d0efeb90c71 100644 (file)
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -326,7 +326,7 @@ typedef struct NBDClient NBDClient;
 
 NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
                           uint64_t size, const char *name, const char *desc,
-                          const char *bitmap, uint16_t nbdflags,
+                          const char *bitmap, uint16_t nbdflags, bool shared,
                           void (*close)(NBDExport *), bool writethrough,
                           BlockBackend *on_eject_blk, Error **errp);
 void nbd_export_close(NBDExport *exp);

diff --git a/nbd/server.c b/nbd/server.c
index f55ccf8edfde9509b026a0bd8e3db934d11fc9b0..0fb41c6c50eae27ec7b4f7f9529e7f9896747aec 100644 (file)
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1461,7 +1461,7 @@ static void nbd_eject_notifier(Notifier *n, void *data)
 
 NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
                           uint64_t size, const char *name, const char *desc,
-                          const char *bitmap, uint16_t nbdflags,
+                          const char *bitmap, uint16_t nbdflags, bool shared,
                           void (*close)(NBDExport *), bool writethrough,
                           BlockBackend *on_eject_blk, Error **errp)
 {
@@ -1487,6 +1487,8 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
     perm = BLK_PERM_CONSISTENT_READ;
     if ((nbdflags & NBD_FLAG_READ_ONLY) == 0) {
         perm |= BLK_PERM_WRITE;
+    } else if (shared) {
+        nbdflags |= NBD_FLAG_CAN_MULTI_CONN;
     }
     blk = blk_new(bdrv_get_aio_context(bs), perm,
                   BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 83b6c32d73aa4c0c864a7b2c82d418433131a53b..2403ef3d0f9f02ff1ccb911cf8c531fad70138b2 100644 (file)
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -1173,7 +1173,7 @@ int main(int argc, char **argv)
     }
 
     export = nbd_export_new(bs, dev_offset, fd_size, export_name,
-                            export_description, bitmap, nbdflags,
+                            export_description, bitmap, nbdflags, shared > 1,
                             nbd_export_closed, writethrough, NULL,
                             &error_fatal);
 

diff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out
index d5201b2356a3d3c7dc1230459745543d587e2f32..2bca28ae72f93217bf58807edecf205af0541c31 100644 (file)
--- a/tests/qemu-iotests/223.out
+++ b/tests/qemu-iotests/223.out
@@ -40,7 +40,7 @@ exports available: 0
 exports available: 2
  export: 'n'
   size:  4194304
-  flags: 0x4ef ( readonly flush fua trim zeroes df cache )
+  flags: 0x5ef ( readonly flush fua trim zeroes df multi cache )
   min block: 1
   opt block: 4096
   max block: 33554432