From e31d802479df9daff1994a7ed1e36bbc5bb19d03 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 29 Nov 2018 20:32:30 -0600 Subject: [PATCH 1/3] nbd/server: Advertise all contexts in response to bare LIST The NBD spec, and even our code comment, says that if the client asks for NBD_OPT_LIST_META_CONTEXT with 0 queries, then we should reply with (a possibly-compressed representation of) ALL contexts that we are willing to let them try. But commit 3d068aff forgot to advertise qemu:dirty-bitmap:FOO. Signed-off-by: Eric Blake Message-Id: <20181130023232.3079982-2-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy --- nbd/server.c | 1 + 1 file changed, 1 insertion(+) diff --git a/nbd/server.c b/nbd/server.c index dc04513de7..7af0ddffb2 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -978,6 +978,7 @@ static int nbd_negotiate_meta_queries(NBDClient *client, if (client->opt == NBD_OPT_LIST_META_CONTEXT && !nb_queries) { /* enable all known contexts */ meta->base_allocation = true; + meta->bitmap = !!meta->exp->export_bitmap; } else { for (i = 0; i < nb_queries; ++i) { ret = nbd_negotiate_meta_query(client, meta, errp); From 47829c40794160debdb33b4a042d182e776876d4 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 29 Nov 2018 20:32:31 -0600 Subject: [PATCH 2/3] nbd/client: Make x-dirty-bitmap more reliable The implementation of x-dirty-bitmap in qemu 3.0 (commit 216ee365) silently falls back to treating the server as not supporting NBD_CMD_BLOCK_STATUS if a requested meta_context name was not negotiated, which in turn means treating the _entire_ image as data. Since our hack relied on using 'qemu-img map' to view which portions of the image were dirty by seeing what the redirected bdrv_block_status() treats as holes, this means that our fallback treats the entire image as clean. Better would have been to treat the entire image as dirty, or to fail to connect because the user's request for a specific context could not be honored. This patch goes with the latter. Signed-off-by: Eric Blake Message-Id: <20181130023232.3079982-3-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/nbd-client.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/block/nbd-client.c b/block/nbd-client.c index 76e9ca3abe..e6e27dafa6 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -992,6 +992,11 @@ int nbd_client_init(BlockDriverState *bs, logout("Failed to negotiate with the NBD server\n"); return ret; } + if (x_dirty_bitmap && !client->info.base_allocation) { + error_setg(errp, "requested x-dirty-bitmap %s not found", + x_dirty_bitmap); + return -EINVAL; + } if (client->info.flags & NBD_FLAG_READ_ONLY) { ret = bdrv_apply_auto_read_only(bs, "NBD export is read-only", errp); if (ret < 0) { From c688e6ca7b41a105241054853d250df64addbf8f Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 29 Nov 2018 20:32:32 -0600 Subject: [PATCH 3/3] nbd/client: Send NBD_CMD_DISC if open fails after connect If nbd_client_init() fails after we are already connected, then the server will spam logs with: Disconnect client, due to: Unexpected end-of-file before all bytes were read unless we gracefully disconnect before closing the connection. Ways to trigger this: $ opts=driver=nbd,export=foo,server.type=inet,server.host=localhost,server.port=10809 $ qemu-img map --output=json --image-opts $opts,read-only=off $ qemu-img map --output=json --image-opts $opts,x-dirty-bitmap=nosuch: Signed-off-by: Eric Blake Message-Id: <20181130023232.3079982-4-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/nbd-client.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/block/nbd-client.c b/block/nbd-client.c index e6e27dafa6..fc5b7eda8e 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -995,12 +995,13 @@ int nbd_client_init(BlockDriverState *bs, if (x_dirty_bitmap && !client->info.base_allocation) { error_setg(errp, "requested x-dirty-bitmap %s not found", x_dirty_bitmap); - return -EINVAL; + ret = -EINVAL; + goto fail; } if (client->info.flags & NBD_FLAG_READ_ONLY) { ret = bdrv_apply_auto_read_only(bs, "NBD export is read-only", errp); if (ret < 0) { - return ret; + goto fail; } } if (client->info.flags & NBD_FLAG_SEND_FUA) { @@ -1029,4 +1030,17 @@ int nbd_client_init(BlockDriverState *bs, logout("Established connection with NBD server\n"); return 0; + + fail: + /* + * We have connected, but must fail for other reasons. The + * connection is still blocking; send NBD_CMD_DISC as a courtesy + * to the server. + */ + { + NBDRequest request = { .type = NBD_CMD_DISC }; + + nbd_send_request(client->ioc ?: QIO_CHANNEL(sioc), &request); + return ret; + } }