Skip to content

Commit

Permalink
nbd: Advertise multi-conn for shared read-only connections
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
ebblake committed Sep 5, 2019
1 parent eac2f39 commit 61cc872
Show file tree
Hide file tree
Showing 6 changed files with 8 additions and 5 deletions.
2 changes: 1 addition & 1 deletion blockdev-nbd.c
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions docs/interop/nbd.txt
Expand Up @@ -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
2 changes: 1 addition & 1 deletion include/block/nbd.h
Expand Up @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion nbd/server.c
Expand Up @@ -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)
{
Expand All @@ -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 |
Expand Down
2 changes: 1 addition & 1 deletion qemu-nbd.c
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion tests/qemu-iotests/223.out
Expand Up @@ -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
Expand Down

0 comments on commit 61cc872

Please sign in to comment.