[Qemu-devel] [PATCH v3 0/5] NBD export bitmaps

[Qemu-devel] [PATCH v3 0/5] NBD export bitmaps

[Vladimir Sementsov-Ogievskiy]

Hi all.

This is a proposal and realization of new NBD meta context:
qemu. (I hope to send corresponding proposal to NBD protocol soon)

New possible queries will look like:
qemu:dirty-bitmap:<export-bitmap-name>

Mapping from export-bitmap-name to BdrvDirtyBitmap is done through qmp
command nbd-server-add-bitmap. For now, only one bitmap export is
allowed per NBD export, however it may be easily improved if needed 
(we don't have such cases for now)

Client and testing.
I wrote client code for Virtuozzo, but it turned out to be unused,
actually it's used only for tests. We don't have cases, where we need
to import dirty bitmap through qemu nbd-client. All this done for
exporting dirty bitmaps to the third tool. So, I think, it is not worth
refactoring, rebasing and merging client part upstream, if there are no
real usage cases.

v3:
01: new
02: rewritten to satisfy changes in 03, drop r-b
03: - fix comments
    - rewrite nbd_meta_bitmap_query() and rename it to
      nbd_meta_qemu_query(): parse 'qemu:dirty-bitmaps:' for LIST
      option to represent list of all dirty-bitmap contexts.
    - trace points
    - s/512/BDRV_SECTOR_SIZE/
      drop TODO comment
04: s/2.13/3.0
05: new

v2:
01 from v1 is dropped: actually, we don't need generic namespace
parsing for now (especially, after moving to qemu: namespace, which has
the same length as base:), lets postpone it.

01: Improve comment wording (Eric), add Eric's r-b
02: improve commit message
    move NBD_STATE_DIRTY to header
    add comment on NBD_MAX_BITMAP_EXTENTS
    remove MAX_EXTENT_LENGTH and instead update add_extents() which
      uses it
    use export_bitmap_context instead of export_bitmap_name to reduce
      operations on it
    move from qemu-dirty-bitmap to qemu:dirty-bitmap
    other way to parse namespace name
    handle FLAG_DF
03: Improve specification of new qmp command (Eric)

Vladimir Sementsov-Ogievskiy (5):
  nbd/server: fix trace
  nbd/server: add nbd_meta_empty_or_pattern helper
  nbd/server: implement dirty bitmap export
  qapi: new qmp command nbd-server-add-bitmap
  docs/interop: add nbd.txt

 docs/interop/nbd.txt |  37 ++++++
 qapi/block.json      |  23 ++++
 include/block/nbd.h  |   6 +
 blockdev-nbd.c       |  23 ++++
 nbd/server.c         | 322 ++++++++++++++++++++++++++++++++++++++++++++++-----
 MAINTAINERS          |   1 +
 6 files changed, 383 insertions(+), 29 deletions(-)
 create mode 100644 docs/interop/nbd.txt

-- 
2.11.1

[Qemu-devel] [PATCH v3 1/5] nbd/server: fix trace

[Vladimir Sementsov-Ogievskiy]

Return code = 1 doesn't mean that we parsed base:allocation. Move
trace point to appropriate place.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 nbd/server.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index 9e1f227178..7584ff7dcc 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -741,7 +741,10 @@ static int nbd_negotiate_send_meta_context(NBDClient *client,
  * the current name, after the 'base:' portion has been stripped.
  *
  * Return -errno on I/O error, 0 if option was completely handled by
- * sending a reply about inconsistent lengths, or 1 on success. */
+ * sending a reply about inconsistent lengths, or 1 on success.
+ *
+ * Note: return code = 1 doesn't mean that we've parsed "base:allocation"
+ * namespace. It only means that there are no errors.*/
 static int nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta,
                                uint32_t len, Error **errp)
 {
@@ -768,10 +771,10 @@ static int nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta,
     }
 
     if (strncmp(query, "allocation", alen) == 0) {
+        trace_nbd_negotiate_meta_query_parse("base:allocation");
         meta->base_allocation = true;
     }
 
-    trace_nbd_negotiate_meta_query_parse("base:allocation");
     return 1;
 }
 
-- 
2.11.1

Re: [Qemu-devel] [PATCH v3 1/5] nbd/server: fix trace

[Eric Blake]

On 05/23/2018 05:24 AM, Vladimir Sementsov-Ogievskiy wrote:
> Return code = 1 doesn't mean that we parsed base:allocation. Move
> trace point to appropriate place.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   nbd/server.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 

> @@ -768,10 +771,10 @@ static int nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta,
>       }
>   
>       if (strncmp(query, "allocation", alen) == 0) {
> +        trace_nbd_negotiate_meta_query_parse("base:allocation");
>           meta->base_allocation = true;
>       }

It's probably worth tracing even when we successfully skip the request, 
as in:

if (strncmp...) {
     trace_nbd_negotiate_meta_query_parse("base:allocation");
     meta->base_allocation = true;
} else {
     trace_nbd_negotiate_meta_query_skip("unrecognized request %s", query);
}

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [PATCH v3 2/5] nbd/server: add nbd_meta_empty_or_pattern helper

[Vladimir Sementsov-Ogievskiy]

Add nbd_meta_pattern() and nbd_meta_empty_or_pattern() helpers for
metadata query parsing. nbd_meta_pattern() will be reused for "qemu"
namespace in following patches.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 nbd/server.c | 82 ++++++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 57 insertions(+), 25 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index 7584ff7dcc..8869dfe1f9 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -734,48 +734,79 @@ static int nbd_negotiate_send_meta_context(NBDClient *client,
     return qio_channel_writev_all(client->ioc, iov, 2, errp) < 0 ? -EIO : 0;
 }
 
-/* nbd_meta_base_query
- *
- * Handle query to 'base' namespace. For now, only base:allocation context is
- * available in it.  'len' is the amount of text remaining to be read from
- * the current name, after the 'base:' portion has been stripped.
+/* Read strlen(@pattern) bytes, and set @match to true if they match @pattern.
+ * @match is never set to false.
  *
  * Return -errno on I/O error, 0 if option was completely handled by
  * sending a reply about inconsistent lengths, or 1 on success.
  *
- * Note: return code = 1 doesn't mean that we've parsed "base:allocation"
- * namespace. It only means that there are no errors.*/
-static int nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta,
-                               uint32_t len, Error **errp)
+ * Note: return code = 1 doesn't mean that we've read exactly @pattern
+ * It only means that there are no errors. */
+static int nbd_meta_pattern(NBDClient *client, const char *pattern, bool *match,
+                            Error **errp)
 {
     int ret;
-    char query[sizeof("allocation") - 1];
-    size_t alen = strlen("allocation");
+    char *query;
+    int len = strlen(pattern);
+
+    assert(len);
+
+    query = g_malloc(len);
+    ret = nbd_opt_read(client, query, len, errp);
+    if (ret <= 0) {
+        g_free(query);
+        return ret;
+    }
+
+    if (strncmp(query, pattern, len) == 0) {
+        trace_nbd_negotiate_meta_query_parse(pattern);
+        *match = true;
+    }
+    g_free(query);
 
+    return 1;
+}
+
+/* Read @len bytes, and set @match to true if they match @pattern, or if @len
+ * is 0 and the client is performing _LIST_. @match is never set to false.
+ *
+ * Return -errno on I/O error, 0 if option was completely handled by
+ * sending a reply about inconsistent lengths, or 1 on success.
+ *
+ * Note: return code = 1 doesn't mean that we've read exactly @pattern
+ * It only means that there are no errors. */
+static int nbd_meta_empty_or_pattern(NBDClient *client, const char *pattern,
+                                     uint32_t len, bool *match, Error **errp)
+{
     if (len == 0) {
         if (client->opt == NBD_OPT_LIST_META_CONTEXT) {
-            meta->base_allocation = true;
+            *match = true;
         }
-        trace_nbd_negotiate_meta_query_parse("base:");
+        trace_nbd_negotiate_meta_query_parse("empty");
         return 1;
     }
 
-    if (len != alen) {
-        trace_nbd_negotiate_meta_query_skip("not base:allocation");
+    if (len != strlen(pattern)) {
+        trace_nbd_negotiate_meta_query_skip("different lengths");
         return nbd_opt_skip(client, len, errp);
     }
 
-    ret = nbd_opt_read(client, query, len, errp);
-    if (ret <= 0) {
-        return ret;
-    }
-
-    if (strncmp(query, "allocation", alen) == 0) {
-        trace_nbd_negotiate_meta_query_parse("base:allocation");
-        meta->base_allocation = true;
-    }
+    return nbd_meta_pattern(client, pattern, match, errp);
+}
 
-    return 1;
+/* nbd_meta_base_query
+ *
+ * Handle query to 'base' namespace. For now, only base:allocation context is
+ * available in it.  'len' is the amount of text remaining to be read from
+ * the current name, after the 'base:' portion has been stripped.
+ *
+ * Return -errno on I/O error, 0 if option was completely handled by
+ * sending a reply about inconsistent lengths, or 1 on success. */
+static int nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta,
+                               uint32_t len, Error **errp)
+{
+    return nbd_meta_empty_or_pattern(client, "allocation", len,
+                                     &meta->base_allocation, errp);
 }
 
 /* nbd_negotiate_meta_query
@@ -821,6 +852,7 @@ static int nbd_negotiate_meta_query(NBDClient *client,
         return nbd_opt_skip(client, len, errp);
     }
 
+    trace_nbd_negotiate_meta_query_parse("base:");
     return nbd_meta_base_query(client, meta, len, errp);
 }
 
-- 
2.11.1

[Qemu-devel] [PATCH v3 3/5] nbd/server: implement dirty bitmap export

[Vladimir Sementsov-Ogievskiy]

Handle new NBD meta namespace: "qemu", and corresponding queries:
"qemu:dirty-bitmap:<export bitmap name>".

With new metadata context negotiated, BLOCK_STATUS query will reply
with dirty-bitmap data, converted to extents. New public function
nbd_export_bitmap selects bitmap to export. For now, only one bitmap
may be exported.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/block/nbd.h |   6 ++
 nbd/server.c        | 265 ++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 253 insertions(+), 18 deletions(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index fcdcd54502..a653d0fba7 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -234,6 +234,10 @@ enum {
 #define NBD_STATE_HOLE (1 << 0)
 #define NBD_STATE_ZERO (1 << 1)
 
+/* Flags for extents (NBDExtent.flags) of NBD_REPLY_TYPE_BLOCK_STATUS,
+ * for qemu:dirty-bitmap:* meta contexts */
+#define NBD_STATE_DIRTY (1 << 0)
+
 static inline bool nbd_reply_type_is_error(int type)
 {
     return type & (1 << 15);
@@ -315,6 +319,8 @@ void nbd_client_put(NBDClient *client);
 void nbd_server_start(SocketAddress *addr, const char *tls_creds,
                       Error **errp);
 
+void nbd_export_bitmap(NBDExport *exp, const char *bitmap,
+                       const char *bitmap_export_name, Error **errp);
 
 /* nbd_read
  * Reads @size bytes from @ioc. Returns 0 on success.
diff --git a/nbd/server.c b/nbd/server.c
index 8869dfe1f9..df1c581aeb 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -23,6 +23,12 @@
 #include "nbd-internal.h"
 
 #define NBD_META_ID_BASE_ALLOCATION 0
+#define NBD_META_ID_DIRTY_BITMAP 1
+
+/* NBD_MAX_BITMAP_EXTENTS: 1 mb of extents data. An empirical constant. If need
+ * to increase, note that NBD protocol defines 32 mb as a limit, after which the
+ * reply may be considered as a denial of service attack. */
+#define NBD_MAX_BITMAP_EXTENTS (0x100000 / 8)
 
 static int system_errno_to_nbd_errno(int err)
 {
@@ -80,6 +86,9 @@ struct NBDExport {
 
     BlockBackend *eject_notifier_blk;
     Notifier eject_notifier;
+
+    BdrvDirtyBitmap *export_bitmap;
+    char *export_bitmap_context;
 };
 
 static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports);
@@ -92,6 +101,7 @@ typedef struct NBDExportMetaContexts {
     bool valid; /* means that negotiation of the option finished without
                    errors */
     bool base_allocation; /* export base:allocation context (block status) */
+    bool dirty_bitmap; /* export qemu:dirty-bitmap:<export bitmap name> */
 } NBDExportMetaContexts;
 
 struct NBDClient {
@@ -809,6 +819,55 @@ static int nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta,
                                      &meta->base_allocation, errp);
 }
 
+/* nbd_meta_bitmap_query
+ *
+ * Handle query to 'qemu:' namespace.
+ * @len is the amount of text remaining to be read from the current name, after
+ * the 'qemu:' portion has been stripped.
+ *
+ * Return -errno on I/O error, 0 if option was completely handled by
+ * sending a reply about inconsistent lengths, or 1 on success. */
+static int nbd_meta_qemu_query(NBDClient *client, NBDExportMetaContexts *meta,
+                               uint32_t len, Error **errp)
+{
+    bool dirty_bitmap = false;
+    int dirty_bitmap_len = strlen("dirty-bitmap:");
+    int ret;
+
+    if (!client->exp->export_bitmap) {
+        return nbd_opt_skip(client, len, errp);
+    }
+
+    if (len == 0) {
+        if (client->opt == NBD_OPT_LIST_META_CONTEXT) {
+            meta->dirty_bitmap = true;
+        }
+        trace_nbd_negotiate_meta_query_parse("empty");
+        return 1;
+    }
+
+    if (len < dirty_bitmap_len) {
+        trace_nbd_negotiate_meta_query_skip("not dirty-bitmap:");
+        return nbd_opt_skip(client, len, errp);
+    }
+
+    len -= dirty_bitmap_len;
+    ret = nbd_meta_pattern(client, "dirty-bitmap:", &dirty_bitmap, errp);
+    if (ret <= 0) {
+        return ret;
+    }
+    if (!dirty_bitmap) {
+        trace_nbd_negotiate_meta_query_skip("not dirty-bitmap:");
+        return nbd_opt_skip(client, len, errp);
+    }
+
+    trace_nbd_negotiate_meta_query_parse("dirty-bitmap:");
+
+    return nbd_meta_empty_or_pattern(
+            client, client->exp->export_bitmap_context +
+            strlen("qemu:dirty_bitmap:"), len, &meta->dirty_bitmap, errp);
+}
+
 /* nbd_negotiate_meta_query
  *
  * Parse namespace name and call corresponding function to parse body of the
@@ -825,8 +884,10 @@ static int nbd_negotiate_meta_query(NBDClient *client,
                                     NBDExportMetaContexts *meta, Error **errp)
 {
     int ret;
-    char query[sizeof("base:") - 1];
-    size_t baselen = strlen("base:");
+    size_t ns_len = 5;
+    char ns[5]; /* Both 'qemu' and 'base' namespaces have length = 5 including a
+                   colon. If it's needed to introduce a namespace of the other
+                   length, this should be certainly refactored. */
     uint32_t len;
 
     ret = nbd_opt_read(client, &len, sizeof(len), errp);
@@ -835,25 +896,27 @@ static int nbd_negotiate_meta_query(NBDClient *client,
     }
     cpu_to_be32s(&len);
 
-    /* The only supported namespace for now is 'base'. So query should start
-     * with 'base:'. Otherwise, we can ignore it and skip the remainder. */
-    if (len < baselen) {
+    if (len < ns_len) {
         trace_nbd_negotiate_meta_query_skip("length too short");
         return nbd_opt_skip(client, len, errp);
     }
 
-    len -= baselen;
-    ret = nbd_opt_read(client, query, baselen, errp);
+    len -= ns_len;
+    ret = nbd_opt_read(client, ns, ns_len, errp);
     if (ret <= 0) {
         return ret;
     }
-    if (strncmp(query, "base:", baselen) != 0) {
-        trace_nbd_negotiate_meta_query_skip("not for base: namespace");
-        return nbd_opt_skip(client, len, errp);
+
+    if (!strncmp(ns, "base:", ns_len)) {
+        trace_nbd_negotiate_meta_query_parse("base:");
+        return nbd_meta_base_query(client, meta, len, errp);
+    } else if (!strncmp(ns, "qemu:", ns_len)) {
+        trace_nbd_negotiate_meta_query_parse("qemu:");
+        return nbd_meta_qemu_query(client, meta, len, errp);
     }
 
-    trace_nbd_negotiate_meta_query_parse("base:");
-    return nbd_meta_base_query(client, meta, len, errp);
+    trace_nbd_negotiate_meta_query_skip("unknown namespace");
+    return nbd_opt_skip(client, len, errp);
 }
 
 /* nbd_negotiate_meta_queries
@@ -923,6 +986,16 @@ static int nbd_negotiate_meta_queries(NBDClient *client,
         }
     }
 
+    if (meta->dirty_bitmap) {
+        ret = nbd_negotiate_send_meta_context(client,
+                                              exp->export_bitmap_context,
+                                              NBD_META_ID_DIRTY_BITMAP,
+                                              errp);
+        if (ret < 0) {
+            return ret;
+        }
+    }
+
     ret = nbd_negotiate_send_rep(client, NBD_REP_ACK, errp);
     if (ret == 0) {
         meta->valid = true;
@@ -1551,6 +1624,11 @@ void nbd_export_put(NBDExport *exp)
             exp->blk = NULL;
         }
 
+        if (exp->export_bitmap) {
+            bdrv_dirty_bitmap_set_qmp_locked(exp->export_bitmap, false);
+            g_free(exp->export_bitmap_context);
+        }
+
         g_free(exp);
     }
 }
@@ -1792,6 +1870,9 @@ static int blockstatus_to_extent_be(BlockDriverState *bs, uint64_t offset,
 }
 
 /* nbd_co_send_extents
+ *
+ * NBD_REPLY_FLAG_DONE is not set, don't forget to send it.
+ *
  * @extents should be in big-endian */
 static int nbd_co_send_extents(NBDClient *client, uint64_t handle,
                                NBDExtent *extents, unsigned nb_extents,
@@ -1804,7 +1885,7 @@ static int nbd_co_send_extents(NBDClient *client, uint64_t handle,
         {.iov_base = extents, .iov_len = nb_extents * sizeof(extents[0])}
     };
 
-    set_be_chunk(&chunk.h, NBD_REPLY_FLAG_DONE, NBD_REPLY_TYPE_BLOCK_STATUS,
+    set_be_chunk(&chunk.h, 0, NBD_REPLY_TYPE_BLOCK_STATUS,
                  handle, sizeof(chunk) - sizeof(chunk.h) + iov[1].iov_len);
     stl_be_p(&chunk.context_id, context_id);
 
@@ -1829,6 +1910,91 @@ static int nbd_co_send_block_status(NBDClient *client, uint64_t handle,
     return nbd_co_send_extents(client, handle, &extent, 1, context_id, errp);
 }
 
+/* Set several extents, describing region of given @length with given @flags.
+ * Do not set more than @nb_extents, return number of set extents.
+ */
+static unsigned add_extents(NBDExtent *extents, unsigned nb_extents,
+                            uint64_t length, uint32_t flags)
+{
+    unsigned i = 0;
+    uint32_t max_extent = QEMU_ALIGN_DOWN(INT32_MAX, BDRV_SECTOR_SIZE);
+    uint32_t max_extent_be = cpu_to_be32(max_extent);
+    uint32_t flags_be = cpu_to_be32(flags);
+
+    for (i = 0; i < nb_extents && length > max_extent;
+         i++, length -= max_extent)
+    {
+        extents[i].length = max_extent_be;
+        extents[i].flags = flags_be;
+    }
+
+    if (length > 0 && i < nb_extents) {
+        extents[i].length = cpu_to_be32(length);
+        extents[i].flags = flags_be;
+        i++;
+    }
+
+    return i;
+}
+
+static unsigned bitmap_to_extents(BdrvDirtyBitmap *bitmap, uint64_t offset,
+                                  uint64_t length, NBDExtent *extents,
+                                  unsigned nb_extents)
+{
+    uint64_t begin = offset, end;
+    uint64_t overall_end = offset + length;
+    unsigned i = 0;
+    BdrvDirtyBitmapIter *it;
+    bool dirty;
+
+    bdrv_dirty_bitmap_lock(bitmap);
+
+    it = bdrv_dirty_iter_new(bitmap);
+    dirty = bdrv_get_dirty_locked(NULL, bitmap, offset);
+
+    while (begin < overall_end && i < nb_extents) {
+        if (dirty) {
+            end = bdrv_dirty_bitmap_next_zero(bitmap, begin);
+        } else {
+            bdrv_set_dirty_iter(it, begin);
+            end = bdrv_dirty_iter_next(it);
+        }
+        if (end == -1) {
+            end = overall_end;
+        }
+
+        i += add_extents(extents + i, nb_extents - i, end - begin,
+                         dirty ? NBD_STATE_DIRTY : 0);
+        begin = end;
+        dirty = !dirty;
+    }
+
+    bdrv_dirty_iter_free(it);
+
+    bdrv_dirty_bitmap_unlock(bitmap);
+
+    return i;
+}
+
+static int nbd_co_send_bitmap(NBDClient *client, uint64_t handle,
+                              BdrvDirtyBitmap *bitmap, uint64_t offset,
+                              uint64_t length, bool dont_fragment,
+                              uint32_t context_id, Error **errp)
+{
+    int ret;
+    unsigned nb_extents = dont_fragment ? 1 : NBD_MAX_BITMAP_EXTENTS;
+    NBDExtent *extents = g_new(NBDExtent, nb_extents);
+
+    nb_extents = bitmap_to_extents(bitmap, offset, length, extents, nb_extents);
+
+    ret = nbd_co_send_extents(client, handle, extents, nb_extents, context_id,
+                              errp);
+
+    g_free(extents);
+
+    return ret;
+}
+
 /* nbd_co_receive_request
  * Collect a client request. Return 0 if request looks valid, -EIO to drop
  * connection right away, and any other negative value to report an error to
@@ -2042,11 +2208,33 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
                                       "discard failed", errp);
 
     case NBD_CMD_BLOCK_STATUS:
-        if (client->export_meta.valid && client->export_meta.base_allocation) {
-            return nbd_co_send_block_status(client, request->handle,
-                                            blk_bs(exp->blk), request->from,
-                                            request->len,
-                                            NBD_META_ID_BASE_ALLOCATION, errp);
+        if (client->export_meta.valid &&
+            (client->export_meta.base_allocation ||
+             client->export_meta.dirty_bitmap))
+        {
+            if (client->export_meta.base_allocation) {
+                ret = nbd_co_send_block_status(client, request->handle,
+                                               blk_bs(exp->blk), request->from,
+                                               request->len,
+                                               NBD_META_ID_BASE_ALLOCATION,
+                                               errp);
+                if (ret < 0) {
+                    return ret;
+                }
+            }
+
+            if (client->export_meta.dirty_bitmap) {
+                ret = nbd_co_send_bitmap(client, request->handle,
+                                         client->exp->export_bitmap,
+                                         request->from, request->len,
+                                         request->flags & NBD_CMD_FLAG_DF,
+                                         NBD_META_ID_DIRTY_BITMAP, errp);
+                if (ret < 0) {
+                    return ret;
+                }
+            }
+
+            return nbd_co_send_structured_done(client, request->handle, errp);
         } else {
             return nbd_send_generic_reply(client, request->handle, -EINVAL,
                                           "CMD_BLOCK_STATUS not negotiated",
@@ -2198,3 +2386,44 @@ void nbd_client_new(NBDExport *exp,
     co = qemu_coroutine_create(nbd_co_client_start, client);
     qemu_coroutine_enter(co);
 }
+
+void nbd_export_bitmap(NBDExport *exp, const char *bitmap,
+                       const char *bitmap_export_name, Error **errp)
+{
+    BdrvDirtyBitmap *bm = NULL;
+    BlockDriverState *bs = blk_bs(exp->blk);
+
+    if (exp->export_bitmap) {
+        error_setg(errp, "Export bitmap is already set");
+        return;
+    }
+
+    while (true) {
+        bm = bdrv_find_dirty_bitmap(bs, bitmap);
+        if (bm != NULL || bs->backing == NULL) {
+            break;
+        }
+
+        bs = bs->backing->bs;
+    }
+
+    if (bm == NULL) {
+        error_setg(errp, "Bitmap '%s' is not found", bitmap);
+        return;
+    }
+
+    if (bdrv_dirty_bitmap_enabled(bm)) {
+        error_setg(errp, "Bitmap '%s' is enabled", bitmap);
+        return;
+    }
+
+    if (bdrv_dirty_bitmap_qmp_locked(bm)) {
+        error_setg(errp, "Bitmap '%s' is locked", bitmap);
+        return;
+    }
+
+    bdrv_dirty_bitmap_set_qmp_locked(bm, true);
+    exp->export_bitmap = bm;
+    exp->export_bitmap_context =
+            g_strdup_printf("qemu:dirty-bitmap:%s", bitmap_export_name);
+}
-- 
2.11.1

Re: [Qemu-devel] [PATCH v3 3/5] nbd/server: implement dirty bitmap export

[Vladimir Sementsov-Ogievskiy]

23.05.2018 13:24, Vladimir Sementsov-Ogievskiy wrote:
> Handle new NBD meta namespace: "qemu", and corresponding queries:
> "qemu:dirty-bitmap:<export bitmap name>".
>
> With new metadata context negotiated, BLOCK_STATUS query will reply
> with dirty-bitmap data, converted to extents. New public function
> nbd_export_bitmap selects bitmap to export. For now, only one bitmap
> may be exported.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---

[...]

>   
> +/* nbd_meta_bitmap_query
> + *
> + * Handle query to 'qemu:' namespace.
> + * @len is the amount of text remaining to be read from the current name, after
> + * the 'qemu:' portion has been stripped.
> + *
> + * Return -errno on I/O error, 0 if option was completely handled by
> + * sending a reply about inconsistent lengths, or 1 on success. */
> +static int nbd_meta_qemu_query(NBDClient *client, NBDExportMetaContexts *meta,
> +                               uint32_t len, Error **errp)
> +{
> +    bool dirty_bitmap = false;
> +    int dirty_bitmap_len = strlen("dirty-bitmap:");
> +    int ret;
> +
> +    if (!client->exp->export_bitmap) {

client->exp is not yet set here. meta->exp should be used instead (and 
patch should be added, to move from meta->export_name to meta->exp, I'll 
add it in v4)

> +        return nbd_opt_skip(client, len, errp);
> +    }
> +
> +    if (len == 0) {
> +        if (client->opt == NBD_OPT_LIST_META_CONTEXT) {
> +            meta->dirty_bitmap = true;
> +        }
> +        trace_nbd_negotiate_meta_query_parse("empty");
> +        return 1;
> +    }
> +
> +    if (len < dirty_bitmap_len) {
> +        trace_nbd_negotiate_meta_query_skip("not dirty-bitmap:");
> +        return nbd_opt_skip(client, len, errp);
> +    }
> +
> +    len -= dirty_bitmap_len;
> +    ret = nbd_meta_pattern(client, "dirty-bitmap:", &dirty_bitmap, errp);
> +    if (ret <= 0) {
> +        return ret;
> +    }
> +    if (!dirty_bitmap) {
> +        trace_nbd_negotiate_meta_query_skip("not dirty-bitmap:");
> +        return nbd_opt_skip(client, len, errp);
> +    }
> +
> +    trace_nbd_negotiate_meta_query_parse("dirty-bitmap:");
> +
> +    return nbd_meta_empty_or_pattern(
> +            client, client->exp->export_bitmap_context +

meta->exp

> +            strlen("qemu:dirty_bitmap:"), len, &meta->dirty_bitmap, errp);
> +}
> +
>   

-- 
Best regards,
Vladimir

[Qemu-devel] [PATCH v3 4/5] qapi: new qmp command nbd-server-add-bitmap

[Vladimir Sementsov-Ogievskiy]

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 qapi/block.json | 23 +++++++++++++++++++++++
 blockdev-nbd.c  | 23 +++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/qapi/block.json b/qapi/block.json
index c694524002..ddbca2e286 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -269,6 +269,29 @@
   'data': {'name': 'str', '*mode': 'NbdServerRemoveMode'} }
 
 ##
+# @nbd-server-add-bitmap:
+#
+# Expose a dirty bitmap associated with the selected export. The bitmap search
+# starts at the device attached to the export, and includes all backing files.
+# The exported bitmap is then locked until the NBD export is removed.
+#
+# @name: Export name.
+#
+# @bitmap: Bitmap name to search for.
+#
+# @bitmap-export-name: How the bitmap will be seen by nbd clients
+#                      (default @bitmap)
+#
+# Note: the client must use NBD_OPT_SET_META_CONTEXT with a query of
+# "qemu:dirty-bitmap:NAME" (where NAME matches @bitmap-export-name) to access
+# the exposed bitmap.
+#
+# Since: 3.0
+##
+  { 'command': 'nbd-server-add-bitmap',
+    'data': {'name': 'str', 'bitmap': 'str', '*bitmap-export-name': 'str'} }
+
+##
 # @nbd-server-stop:
 #
 # Stop QEMU's embedded NBD server, and unregister all devices previously
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 65a84739ed..6b0c50732c 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -220,3 +220,26 @@ void qmp_nbd_server_stop(Error **errp)
     nbd_server_free(nbd_server);
     nbd_server = NULL;
 }
+
+void qmp_nbd_server_add_bitmap(const char *name, const char *bitmap,
+                               bool has_bitmap_export_name,
+                               const char *bitmap_export_name,
+                               Error **errp)
+{
+    NBDExport *exp;
+
+    if (!nbd_server) {
+        error_setg(errp, "NBD server not running");
+        return;
+    }
+
+    exp = nbd_export_find(name);
+    if (exp == NULL) {
+        error_setg(errp, "Export '%s' is not found", name);
+        return;
+    }
+
+    nbd_export_bitmap(exp, bitmap,
+                      has_bitmap_export_name ? bitmap_export_name : bitmap,
+                      errp);
+}
-- 
2.11.1

[Qemu-devel] [PATCH v3 5/5] docs/interop: add nbd.txt

[Vladimir Sementsov-Ogievskiy]

Describe new metadata namespace: "qemu".

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 docs/interop/nbd.txt | 37 +++++++++++++++++++++++++++++++++++++
 MAINTAINERS          |  1 +
 2 files changed, 38 insertions(+)
 create mode 100644 docs/interop/nbd.txt

diff --git a/docs/interop/nbd.txt b/docs/interop/nbd.txt
new file mode 100644
index 0000000000..7366269fc0
--- /dev/null
+++ b/docs/interop/nbd.txt
@@ -0,0 +1,37 @@
+Qemu supports NBD protocol, and has internal NBD client (look at
+block/nbd.c), internal NBD server (look at blockdev-nbd.c) as well as
+external NBD server tool - qemu-nbd.c. The common code is placed in
+nbd/*.
+
+NBD protocol is specified here:
+https://github.com/NetworkBlockDevice/nbd/blob/master/doc/proto.md
+
+This following paragraphs describe some specific properties of NBD
+protocol realization in Qemu.
+
+
+= Metadata namespaces =
+
+Qemu supports "base:allocation" metadata context as defined in the NBD
+protocol specification and defines own metadata namespace: "qemu".
+
+
+== "qemu" namespace ==
+
+For now, the only type of metadata context in the namespace is dirty
+bitmap. All available metadata contexts have the following form:
+
+   qemu:dirty-bitmap:<dirty-bitmap-export-name>
+
+Each dirty-bitmap metadata context defines the only one flag for
+extents in reply for NBD_CMD_BLOCK_STATUS:
+
+    bit 0: NBD_STATE_DIRTY, means that the extent is "dirty"
+
+For NBD_OPT_LIST_META_CONTEXT the following queries are supported
+additionally to "qemu:dirty-bitmap:<dirty-bitmap-export-name>":
+
+* "qemu:" : returns list of all available metadata contexts in the
+            namespace.
+* "qemu:dirty-bitmap:" : returns list of all available dirty-bitmap
+                         metadata contexts.
diff --git a/MAINTAINERS b/MAINTAINERS
index e187b1f18f..887b479440 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1923,6 +1923,7 @@ F: nbd/
 F: include/block/nbd*
 F: qemu-nbd.*
 F: blockdev-nbd.c
+F: docs/interop/nbd.txt
 T: git git://repo.or.cz/qemu/ericb.git nbd
 
 NFS
-- 
2.11.1