[Qemu-devel] [PULL 0/3] NBD patches for 3.1-rc4

[Qemu-devel] [PULL 0/3] NBD patches for 3.1-rc4

[Eric Blake]

The following changes since commit 4750e1a888ac3d320607f33b676f299005be98e6:

  Update version for v3.1.0-rc3 release (2018-11-28 17:37:34 +0000)

are available in the Git repository at:

  https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2018-12-03

for you to fetch changes up to c688e6ca7b41a105241054853d250df64addbf8f:

  nbd/client: Send NBD_CMD_DISC if open fails after connect (2018-11-30 14:13:37 -0600)

----------------------------------------------------------------
nbd patches for 2018-12-03

Improve x-dirty-bitmap handling for experimenting with pull mode
incremental backups.

- Eric Blake: 0/3 NBD dirty bitmap cleanups

----------------------------------------------------------------
Eric Blake (3):
      nbd/server: Advertise all contexts in response to bare LIST
      nbd/client: Make x-dirty-bitmap more reliable
      nbd/client: Send NBD_CMD_DISC if open fails after connect

 block/nbd-client.c | 21 ++++++++++++++++++++-
 nbd/server.c       |  1 +
 2 files changed, 21 insertions(+), 1 deletion(-)

-- 
2.17.2

[Qemu-devel] [PULL 1/3] nbd/server: Advertise all contexts in response to bare LIST

[Eric Blake]

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 <eblake@redhat.com>
Message-Id: <20181130023232.3079982-2-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 nbd/server.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/nbd/server.c b/nbd/server.c
index dc04513de70..7af0ddffb20 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);
-- 
2.17.2

[Qemu-devel] [PULL 2/3] nbd/client: Make x-dirty-bitmap more reliable

[Eric Blake]

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 <eblake@redhat.com>
Message-Id: <20181130023232.3079982-3-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/nbd-client.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 76e9ca3abeb..e6e27dafa6a 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) {
-- 
2.17.2

[Qemu-devel] [PULL 3/3] nbd/client: Send NBD_CMD_DISC if open fails after connect

[Eric Blake]

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 <eblake@redhat.com>
Message-Id: <20181130023232.3079982-4-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 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 e6e27dafa6a..fc5b7eda8ee 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;
+    }
 }
-- 
2.17.2

Re: [Qemu-devel] [PULL 0/3] NBD patches for 3.1-rc4

[Peter Maydell]

On Mon, 3 Dec 2018 at 16:46, Eric Blake <eblake@redhat.com> wrote:
>
> The following changes since commit 4750e1a888ac3d320607f33b676f299005be98e6:
>
>   Update version for v3.1.0-rc3 release (2018-11-28 17:37:34 +0000)
>
> are available in the Git repository at:
>
>   https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2018-12-03
>
> for you to fetch changes up to c688e6ca7b41a105241054853d250df64addbf8f:
>
>   nbd/client: Send NBD_CMD_DISC if open fails after connect (2018-11-30 14:13:37 -0600)
>
> ----------------------------------------------------------------
> nbd patches for 2018-12-03
>
> Improve x-dirty-bitmap handling for experimenting with pull mode
> incremental backups.
>
> - Eric Blake: 0/3 NBD dirty bitmap cleanups
>
> ----------------------------------------------------------------
> Eric Blake (3):
>       nbd/server: Advertise all contexts in response to bare LIST
>       nbd/client: Make x-dirty-bitmap more reliable
>       nbd/client: Send NBD_CMD_DISC if open fails after connect
>
>  block/nbd-client.c | 21 ++++++++++++++++++++-
>  nbd/server.c       |  1 +
>  2 files changed, 21 insertions(+), 1 deletion(-)

Applied, thanks.

-- PMM