* [Qemu-devel] [PULL v2 0/9] bitmap export over NBD
@ 2018-06-21 14:57 Eric Blake
2018-06-21 14:57 ` [Qemu-devel] [PULL v2 2/9] nbd/server: Reject 0-length block status request Eric Blake
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Eric Blake @ 2018-06-21 14:57 UTC (permalink / raw)
To: qemu-devel
The following changes since commit 46012db666990ff2eed1d3dc199ab8006439a93b:
Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20180619' into staging (2018-06-20 09:51:30 +0100)
are available in the Git repository at:
git://repo.or.cz/qemu/ericb.git tags/pull-nbd-2018-06-20-v2
for you to fetch changes up to bc37b06a5cde24fb24c2a2cc44dd86756034ba9d:
nbd/server: introduce NBD_CMD_CACHE (2018-06-21 09:41:39 -0500)
Only sending the new patches (2, 9) and the changed patch (6, was 5/7
in v1)
----------------------------------------------------------------
nbd patches for 2018-06-20
Add experimental x-nbd-server-add-bitmap to expose a disabled
bitmap over NBD, in preparation for a pull model incremental
backup scheme. Also fix a corner case protocol issue with
NBD_CMD_BLOCK_STATUS, and add new NBD_CMD_CACHE.
- Eric Blake: tests: Simplify .gitignore
- Eric Blake: nbd/server: Reject 0-length block status request
- Vladimir Sementsov-Ogievskiy: 0/6 NBD export bitmaps
- Vladimir Sementsov-Ogievskiy: nbd/server: introduce NBD_CMD_CACHE
----------------------------------------------------------------
Eric Blake (2):
tests: Simplify .gitignore
nbd/server: Reject 0-length block status request
Vladimir Sementsov-Ogievskiy (7):
nbd/server: fix trace
nbd/server: refactor NBDExportMetaContexts
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
nbd/server: introduce NBD_CMD_CACHE
docs/interop/nbd.txt | 38 +++++
qapi/block.json | 23 +++
include/block/nbd.h | 11 +-
blockdev-nbd.c | 23 +++
nbd/common.c | 2 +
nbd/server.c | 385 ++++++++++++++++++++++++++++++++++++++++++++-------
MAINTAINERS | 1 +
nbd/trace-events | 1 +
tests/.gitignore | 93 +------------
9 files changed, 433 insertions(+), 144 deletions(-)
create mode 100644 docs/interop/nbd.txt
--
2.14.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PULL v2 2/9] nbd/server: Reject 0-length block status request
2018-06-21 14:57 [Qemu-devel] [PULL v2 0/9] bitmap export over NBD Eric Blake
@ 2018-06-21 14:57 ` Eric Blake
2018-06-21 14:57 ` [Qemu-devel] [PULL v2 6/9] nbd/server: implement dirty bitmap export Eric Blake
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Eric Blake @ 2018-06-21 14:57 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Paolo Bonzini, open list:Network Block Dev...
The NBD spec says that behavior is unspecified if the client
requests 0 length for block status; but since the structured
reply is documenting as returning a non-zero length, it's
easier to just diagnose this with an EINVAL error than to
figure out what to return.
CC: qemu-stable@nongnu.org
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180621124937.166549-1-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
nbd/server.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/nbd/server.c b/nbd/server.c
index 9e1f2271784..493a926e063 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2007,6 +2007,10 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
"discard failed", errp);
case NBD_CMD_BLOCK_STATUS:
+ if (!request->len) {
+ return nbd_send_generic_reply(client, request->handle, -EINVAL,
+ "need non-zero length", errp);
+ }
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,
--
2.14.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PULL v2 6/9] nbd/server: implement dirty bitmap export
2018-06-21 14:57 [Qemu-devel] [PULL v2 0/9] bitmap export over NBD Eric Blake
2018-06-21 14:57 ` [Qemu-devel] [PULL v2 2/9] nbd/server: Reject 0-length block status request Eric Blake
@ 2018-06-21 14:57 ` Eric Blake
2018-06-21 14:57 ` [Qemu-devel] [PULL v2 9/9] nbd/server: introduce NBD_CMD_CACHE Eric Blake
2018-06-22 9:57 ` [Qemu-devel] [PULL v2 0/9] bitmap export over NBD Peter Maydell
3 siblings, 0 replies; 9+ messages in thread
From: Eric Blake @ 2018-06-21 14:57 UTC (permalink / raw)
To: qemu-devel
Cc: Vladimir Sementsov-Ogievskiy, Paolo Bonzini, Kevin Wolf,
Max Reitz, open list:Network Block Dev...
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Handle a new NBD meta namespace: "qemu", and corresponding queries:
"qemu:dirty-bitmap:<export bitmap name>".
With the new metadata context negotiated, BLOCK_STATUS query will reply
with dirty-bitmap data, converted to extents. The new public function
nbd_export_bitmap selects which bitmap to export. For now, only one bitmap
may be exported.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20180609151758.17343-5-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: wording tweaks, minor cleanups, additional tracing]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
include/block/nbd.h | 8 +-
nbd/server.c | 278 +++++++++++++++++++++++++++++++++++++++++++++++-----
nbd/trace-events | 1 +
3 files changed, 262 insertions(+), 25 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index fcdcd545023..8bb9606c39b 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -229,11 +229,13 @@ enum {
#define NBD_REPLY_TYPE_ERROR NBD_REPLY_ERR(1)
#define NBD_REPLY_TYPE_ERROR_OFFSET NBD_REPLY_ERR(2)
-/* Flags for extents (NBDExtent.flags) of NBD_REPLY_TYPE_BLOCK_STATUS,
- * for base:allocation meta context */
+/* Extent flags for base:allocation in NBD_REPLY_TYPE_BLOCK_STATUS */
#define NBD_STATE_HOLE (1 << 0)
#define NBD_STATE_ZERO (1 << 1)
+/* Extent flags for qemu:dirty-bitmap in NBD_REPLY_TYPE_BLOCK_STATUS */
+#define NBD_STATE_DIRTY (1 << 0)
+
static inline bool nbd_reply_type_is_error(int type)
{
return type & (1 << 15);
@@ -315,6 +317,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 9171cd41680..2c2d62c6361 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -23,6 +23,13 @@
#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 an increase is needed, note that the NBD protocol
+ * recommends no larger than 32 mb, so that the client won't consider
+ * the reply as a denial of service attack. */
+#define NBD_MAX_BITMAP_EXTENTS (0x100000 / 8)
static int system_errno_to_nbd_errno(int err)
{
@@ -80,6 +87,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 +102,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 bitmap; /* export qemu:dirty-bitmap:<export bitmap name> */
} NBDExportMetaContexts;
struct NBDClient {
@@ -814,6 +825,56 @@ 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;
+ size_t dirty_bitmap_len = strlen("dirty-bitmap:");
+ int ret;
+
+ if (!meta->exp->export_bitmap) {
+ trace_nbd_negotiate_meta_query_skip("no dirty-bitmap exported");
+ return nbd_opt_skip(client, len, errp);
+ }
+
+ if (len == 0) {
+ if (client->opt == NBD_OPT_LIST_META_CONTEXT) {
+ meta->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, meta->exp->export_bitmap_context +
+ strlen("qemu:dirty_bitmap:"), len, &meta->bitmap, errp);
+}
+
/* nbd_negotiate_meta_query
*
* Parse namespace name and call corresponding function to parse body of the
@@ -829,9 +890,14 @@ static int nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta,
static int nbd_negotiate_meta_query(NBDClient *client,
NBDExportMetaContexts *meta, Error **errp)
{
+ /*
+ * Both 'qemu' and 'base' namespaces have length = 5 including a
+ * colon. If another length namespace is later introduced, this
+ * should certainly be refactored.
+ */
int ret;
- char query[sizeof("base:") - 1];
- size_t baselen = strlen("base:");
+ size_t ns_len = 5;
+ char ns[5];
uint32_t len;
ret = nbd_opt_read(client, &len, sizeof(len), errp);
@@ -840,25 +906,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
@@ -928,6 +996,16 @@ static int nbd_negotiate_meta_queries(NBDClient *client,
}
}
+ if (meta->bitmap) {
+ ret = nbd_negotiate_send_meta_context(client,
+ meta->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;
@@ -1556,6 +1634,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);
}
}
@@ -1797,9 +1880,15 @@ static int blockstatus_to_extent_be(BlockDriverState *bs, uint64_t offset,
}
/* nbd_co_send_extents
- * @extents should be in big-endian */
+ *
+ * @length is only for tracing purposes (and may be smaller or larger
+ * than the client's original request). @last controls whether
+ * NBD_REPLY_FLAG_DONE is sent. @extents should already be in
+ * big-endian format.
+ */
static int nbd_co_send_extents(NBDClient *client, uint64_t handle,
- NBDExtent *extents, unsigned nb_extents,
+ NBDExtent *extents, unsigned int nb_extents,
+ uint64_t length, bool last,
uint32_t context_id, Error **errp)
{
NBDStructuredMeta chunk;
@@ -1809,7 +1898,9 @@ 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,
+ trace_nbd_co_send_extents(handle, nb_extents, context_id, length, last);
+ set_be_chunk(&chunk.h, last ? NBD_REPLY_FLAG_DONE : 0,
+ NBD_REPLY_TYPE_BLOCK_STATUS,
handle, sizeof(chunk) - sizeof(chunk.h) + iov[1].iov_len);
stl_be_p(&chunk.context_id, context_id);
@@ -1819,8 +1910,8 @@ static int nbd_co_send_extents(NBDClient *client, uint64_t handle,
/* Get block status from the exported device and send it to the client */
static int nbd_co_send_block_status(NBDClient *client, uint64_t handle,
BlockDriverState *bs, uint64_t offset,
- uint64_t length, uint32_t context_id,
- Error **errp)
+ uint64_t length, bool last,
+ uint32_t context_id, Error **errp)
{
int ret;
NBDExtent extent;
@@ -1831,7 +1922,84 @@ static int nbd_co_send_block_status(NBDClient *client, uint64_t handle,
client, handle, -ret, "can't get block status", errp);
}
- return nbd_co_send_extents(client, handle, &extent, 1, context_id, errp);
+ return nbd_co_send_extents(client, handle, &extent, 1, length, last,
+ context_id, errp);
+}
+
+/*
+ * Populate @extents from a dirty bitmap. Unless @dont_fragment, the
+ * final extent may exceed the original @length. Store in @length the
+ * byte length encoded (which may be smaller or larger than the
+ * original), and return the number of extents used.
+ */
+static unsigned int bitmap_to_extents(BdrvDirtyBitmap *bitmap, uint64_t offset,
+ uint64_t *length, NBDExtent *extents,
+ unsigned int nb_extents,
+ bool dont_fragment)
+{
+ uint64_t begin = offset, end;
+ uint64_t overall_end = offset + *length;
+ unsigned int 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);
+
+ assert(begin < overall_end && nb_extents);
+ 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 - begin > UINT32_MAX) {
+ /* Cap to an aligned value < 4G beyond begin. */
+ end = MIN(bdrv_dirty_bitmap_size(bitmap),
+ begin + UINT32_MAX + 1 -
+ bdrv_dirty_bitmap_granularity(bitmap));
+ }
+ if (dont_fragment && end > overall_end) {
+ end = overall_end;
+ }
+
+ extents[i].length = cpu_to_be32(end - begin);
+ extents[i].flags = cpu_to_be32(dirty ? NBD_STATE_DIRTY : 0);
+ i++;
+ begin = end;
+ dirty = !dirty;
+ }
+
+ bdrv_dirty_iter_free(it);
+
+ bdrv_dirty_bitmap_unlock(bitmap);
+
+ *length = end - offset;
+ return i;
+}
+
+static int nbd_co_send_bitmap(NBDClient *client, uint64_t handle,
+ BdrvDirtyBitmap *bitmap, uint64_t offset,
+ uint32_t length, bool dont_fragment, bool last,
+ uint32_t context_id, Error **errp)
+{
+ int ret;
+ unsigned int nb_extents = dont_fragment ? 1 : NBD_MAX_BITMAP_EXTENTS;
+ NBDExtent *extents = g_new(NBDExtent, nb_extents);
+ uint64_t final_length = length;
+
+ nb_extents = bitmap_to_extents(bitmap, offset, &final_length, extents,
+ nb_extents, dont_fragment);
+
+ ret = nbd_co_send_extents(client, handle, extents, nb_extents,
+ final_length, last, context_id, errp);
+
+ g_free(extents);
+
+ return ret;
}
/* nbd_co_receive_request
@@ -2051,11 +2219,34 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
return nbd_send_generic_reply(client, request->handle, -EINVAL,
"need non-zero length", errp);
}
- 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.bitmap))
+ {
+ if (client->export_meta.base_allocation) {
+ ret = nbd_co_send_block_status(client, request->handle,
+ blk_bs(exp->blk), request->from,
+ request->len,
+ !client->export_meta.bitmap,
+ NBD_META_ID_BASE_ALLOCATION,
+ errp);
+ if (ret < 0) {
+ return ret;
+ }
+ }
+
+ if (client->export_meta.bitmap) {
+ ret = nbd_co_send_bitmap(client, request->handle,
+ client->exp->export_bitmap,
+ request->from, request->len,
+ request->flags & NBD_CMD_FLAG_REQ_ONE,
+ true, NBD_META_ID_DIRTY_BITMAP, errp);
+ if (ret < 0) {
+ return ret;
+ }
+ }
+
+ return ret;
} else {
return nbd_send_generic_reply(client, request->handle, -EINVAL,
"CMD_BLOCK_STATUS not negotiated",
@@ -2207,3 +2398,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);
+}
diff --git a/nbd/trace-events b/nbd/trace-events
index dee081e7758..5e1d4afe8e6 100644
--- a/nbd/trace-events
+++ b/nbd/trace-events
@@ -64,6 +64,7 @@ nbd_co_send_simple_reply(uint64_t handle, uint32_t error, const char *errname, i
nbd_co_send_structured_done(uint64_t handle) "Send structured reply done: handle = %" PRIu64
nbd_co_send_structured_read(uint64_t handle, uint64_t offset, void *data, size_t size) "Send structured read data reply: handle = %" PRIu64 ", offset = %" PRIu64 ", data = %p, len = %zu"
nbd_co_send_structured_read_hole(uint64_t handle, uint64_t offset, size_t size) "Send structured read hole reply: handle = %" PRIu64 ", offset = %" PRIu64 ", len = %zu"
+nbd_co_send_extents(uint64_t handle, unsigned int extents, uint32_t id, uint64_t length, int last) "Send block status reply: handle = %" PRIu64 ", extents = %u, context = %d (extents cover %" PRIu64 " bytes, last chunk = %d)"
nbd_co_send_structured_error(uint64_t handle, int err, const char *errname, const char *msg) "Send structured error reply: handle = %" PRIu64 ", error = %d (%s), msg = '%s'"
nbd_co_receive_request_decode_type(uint64_t handle, uint16_t type, const char *name) "Decoding type: handle = %" PRIu64 ", type = %" PRIu16 " (%s)"
nbd_co_receive_request_payload_received(uint64_t handle, uint32_t len) "Payload received: handle = %" PRIu64 ", len = %" PRIu32
--
2.14.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PULL v2 9/9] nbd/server: introduce NBD_CMD_CACHE
2018-06-21 14:57 [Qemu-devel] [PULL v2 0/9] bitmap export over NBD Eric Blake
2018-06-21 14:57 ` [Qemu-devel] [PULL v2 2/9] nbd/server: Reject 0-length block status request Eric Blake
2018-06-21 14:57 ` [Qemu-devel] [PULL v2 6/9] nbd/server: implement dirty bitmap export Eric Blake
@ 2018-06-21 14:57 ` Eric Blake
2018-06-22 9:57 ` [Qemu-devel] [PULL v2 0/9] bitmap export over NBD Peter Maydell
3 siblings, 0 replies; 9+ messages in thread
From: Eric Blake @ 2018-06-21 14:57 UTC (permalink / raw)
To: qemu-devel
Cc: Vladimir Sementsov-Ogievskiy, Paolo Bonzini, Kevin Wolf,
Max Reitz, open list:Network Block Dev...
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Handle nbd CACHE command. Just do read, without sending read data back.
Cache mechanism should be done by exported node driver chain.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20180413143156.11409-1-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: fix two missing case labels in switch statements]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
include/block/nbd.h | 3 ++-
nbd/common.c | 2 ++
nbd/server.c | 11 +++++++----
3 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 8bb9606c39b..daaeae61bf9 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -135,6 +135,7 @@ typedef struct NBDExtent {
#define NBD_FLAG_SEND_TRIM (1 << 5) /* Send TRIM (discard) */
#define NBD_FLAG_SEND_WRITE_ZEROES (1 << 6) /* Send WRITE_ZEROES */
#define NBD_FLAG_SEND_DF (1 << 7) /* Send DF (Do not Fragment) */
+#define NBD_FLAG_SEND_CACHE (1 << 8) /* Send CACHE (prefetch) */
/* New-style handshake (global) flags, sent from server to client, and
control what will happen during handshake phase. */
@@ -195,7 +196,7 @@ enum {
NBD_CMD_DISC = 2,
NBD_CMD_FLUSH = 3,
NBD_CMD_TRIM = 4,
- /* 5 reserved for failed experiment NBD_CMD_CACHE */
+ NBD_CMD_CACHE = 5,
NBD_CMD_WRITE_ZEROES = 6,
NBD_CMD_BLOCK_STATUS = 7,
};
diff --git a/nbd/common.c b/nbd/common.c
index 8c95c1d606e..41f5ed8d9fa 100644
--- a/nbd/common.c
+++ b/nbd/common.c
@@ -148,6 +148,8 @@ const char *nbd_cmd_lookup(uint16_t cmd)
return "flush";
case NBD_CMD_TRIM:
return "trim";
+ case NBD_CMD_CACHE:
+ return "cache";
case NBD_CMD_WRITE_ZEROES:
return "write zeroes";
case NBD_CMD_BLOCK_STATUS:
diff --git a/nbd/server.c b/nbd/server.c
index 2c2d62c6361..274604609f4 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1252,7 +1252,7 @@ static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp)
int ret;
const uint16_t myflags = (NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM |
NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA |
- NBD_FLAG_SEND_WRITE_ZEROES);
+ NBD_FLAG_SEND_WRITE_ZEROES | NBD_FLAG_SEND_CACHE);
bool oldStyle;
/* Old style negotiation header, no room for options
@@ -2034,7 +2034,9 @@ static int nbd_co_receive_request(NBDRequestData *req, NBDRequest *request,
return -EIO;
}
- if (request->type == NBD_CMD_READ || request->type == NBD_CMD_WRITE) {
+ if (request->type == NBD_CMD_READ || request->type == NBD_CMD_WRITE ||
+ request->type == NBD_CMD_CACHE)
+ {
if (request->len > NBD_MAX_BUFFER_SIZE) {
error_setg(errp, "len (%" PRIu32" ) is larger than max len (%u)",
request->len, NBD_MAX_BUFFER_SIZE);
@@ -2119,7 +2121,7 @@ static coroutine_fn int nbd_do_cmd_read(NBDClient *client, NBDRequest *request,
int ret;
NBDExport *exp = client->exp;
- assert(request->type == NBD_CMD_READ);
+ assert(request->type == NBD_CMD_READ || request->type == NBD_CMD_CACHE);
/* XXX: NBD Protocol only documents use of FUA with WRITE */
if (request->flags & NBD_CMD_FLAG_FUA) {
@@ -2138,7 +2140,7 @@ static coroutine_fn int nbd_do_cmd_read(NBDClient *client, NBDRequest *request,
ret = blk_pread(exp->blk, request->from + exp->dev_offset, data,
request->len);
- if (ret < 0) {
+ if (ret < 0 || request->type == NBD_CMD_CACHE) {
return nbd_send_generic_reply(client, request->handle, ret,
"reading from file failed", errp);
}
@@ -2171,6 +2173,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
switch (request->type) {
case NBD_CMD_READ:
+ case NBD_CMD_CACHE:
return nbd_do_cmd_read(client, request, data, errp);
case NBD_CMD_WRITE:
--
2.14.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PULL v2 0/9] bitmap export over NBD
2018-06-21 14:57 [Qemu-devel] [PULL v2 0/9] bitmap export over NBD Eric Blake
` (2 preceding siblings ...)
2018-06-21 14:57 ` [Qemu-devel] [PULL v2 9/9] nbd/server: introduce NBD_CMD_CACHE Eric Blake
@ 2018-06-22 9:57 ` Peter Maydell
2018-06-22 11:43 ` Peter Maydell
3 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2018-06-22 9:57 UTC (permalink / raw)
To: Eric Blake; +Cc: QEMU Developers
On 21 June 2018 at 15:57, Eric Blake <eblake@redhat.com> wrote:
> The following changes since commit 46012db666990ff2eed1d3dc199ab8006439a93b:
>
> Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20180619' into staging (2018-06-20 09:51:30 +0100)
>
> are available in the Git repository at:
>
> git://repo.or.cz/qemu/ericb.git tags/pull-nbd-2018-06-20-v2
>
> for you to fetch changes up to bc37b06a5cde24fb24c2a2cc44dd86756034ba9d:
>
> nbd/server: introduce NBD_CMD_CACHE (2018-06-21 09:41:39 -0500)
>
> Only sending the new patches (2, 9) and the changed patch (6, was 5/7
> in v1)
>
> ----------------------------------------------------------------
> nbd patches for 2018-06-20
>
> Add experimental x-nbd-server-add-bitmap to expose a disabled
> bitmap over NBD, in preparation for a pull model incremental
> backup scheme. Also fix a corner case protocol issue with
> NBD_CMD_BLOCK_STATUS, and add new NBD_CMD_CACHE.
>
> - Eric Blake: tests: Simplify .gitignore
> - Eric Blake: nbd/server: Reject 0-length block status request
> - Vladimir Sementsov-Ogievskiy: 0/6 NBD export bitmaps
> - Vladimir Sementsov-Ogievskiy: nbd/server: introduce NBD_CMD_CACHE
>
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PULL v2 0/9] bitmap export over NBD
2018-06-22 9:57 ` [Qemu-devel] [PULL v2 0/9] bitmap export over NBD Peter Maydell
@ 2018-06-22 11:43 ` Peter Maydell
2018-06-22 12:12 ` Eric Blake
0 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2018-06-22 11:43 UTC (permalink / raw)
To: Eric Blake; +Cc: QEMU Developers
On 22 June 2018 at 10:57, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 21 June 2018 at 15:57, Eric Blake <eblake@redhat.com> wrote:
>> ----------------------------------------------------------------
>> nbd patches for 2018-06-20
>>
>> Add experimental x-nbd-server-add-bitmap to expose a disabled
>> bitmap over NBD, in preparation for a pull model incremental
>> backup scheme. Also fix a corner case protocol issue with
>> NBD_CMD_BLOCK_STATUS, and add new NBD_CMD_CACHE.
>>
>> - Eric Blake: tests: Simplify .gitignore
>> - Eric Blake: nbd/server: Reject 0-length block status request
>> - Vladimir Sementsov-Ogievskiy: 0/6 NBD export bitmaps
>> - Vladimir Sementsov-Ogievskiy: nbd/server: introduce NBD_CMD_CACHE
>>
> Applied, thanks.
...patchew seems to be unhappy, though, eg
https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg06559.html
has an unrelated patchset failing with:
/tmp/qemu-test/src/nbd/server.c: In function 'nbd_trip':
/tmp/qemu-test/src/nbd/server.c:1980:19: error: 'end' may be used uninitialized
in this function [-Werror=maybe-uninitialized]
*length = end - offset;
~~~~^~~~~~~~
/tmp/qemu-test/src/nbd/server.c:1940:30: note: 'end' was declared here
uint64_t begin = offset, end;
^~~
cc1: all warnings being treated as errors
thanks
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PULL v2 0/9] bitmap export over NBD
2018-06-22 11:43 ` Peter Maydell
@ 2018-06-22 12:12 ` Eric Blake
2018-06-22 12:13 ` Peter Maydell
0 siblings, 1 reply; 9+ messages in thread
From: Eric Blake @ 2018-06-22 12:12 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers
On 06/22/2018 06:43 AM, Peter Maydell wrote:
> On 22 June 2018 at 10:57, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 21 June 2018 at 15:57, Eric Blake <eblake@redhat.com> wrote:
>>> ----------------------------------------------------------------
>>> nbd patches for 2018-06-20
>>>
>>> Add experimental x-nbd-server-add-bitmap to expose a disabled
>>> bitmap over NBD, in preparation for a pull model incremental
>>> backup scheme. Also fix a corner case protocol issue with
>>> NBD_CMD_BLOCK_STATUS, and add new NBD_CMD_CACHE.
>>>
>>> - Eric Blake: tests: Simplify .gitignore
>>> - Eric Blake: nbd/server: Reject 0-length block status request
>>> - Vladimir Sementsov-Ogievskiy: 0/6 NBD export bitmaps
>>> - Vladimir Sementsov-Ogievskiy: nbd/server: introduce NBD_CMD_CACHE
>>>
>> Applied, thanks.
>
> ...patchew seems to be unhappy, though, eg
> https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg06559.html
> has an unrelated patchset failing with:
>
> /tmp/qemu-test/src/nbd/server.c: In function 'nbd_trip':
> /tmp/qemu-test/src/nbd/server.c:1980:19: error: 'end' may be used uninitialized
> in this function [-Werror=maybe-uninitialized]
> *length = end - offset;
> ~~~~^~~~~~~~
Uurgh. It's a false positive (the compiler is complaining that the
variable is uninitialized, which can only happen if the while loop is
not executed; but the preconditions guarantee the loop executes at least
once). The assert() that I added was enough to silence gcc 7.3.1 on my
Fedora 27 system, but docker-mingw@fedora is using
gcc-8.1.1-1.fc28.x86_64. This should silence things (another way to
silence would be rewriting while{} into do{}while). I'll submit this as
a formal patch if I can reproduce the problem/fix on docker.
diff --git i/nbd/server.c w/nbd/server.c
index 94137fbfe8f..2379f82d5d4 100644
--- i/nbd/server.c
+++ w/nbd/server.c
@@ -1968,7 +1968,7 @@ static unsigned int
bitmap_to_extents(BdrvDirtyBitmap *bitmap, uint64_t offset,
unsigned int nb_extents,
bool dont_fragment)
{
- uint64_t begin = offset, end;
+ uint64_t begin = offset, end = offset;
uint64_t overall_end = offset + *length;
unsigned int i = 0;
BdrvDirtyBitmapIter *it;
@@ -2008,6 +2008,7 @@ static unsigned int
bitmap_to_extents(BdrvDirtyBitmap *bitmap, uint64_t offset,
bdrv_dirty_bitmap_unlock(bitmap);
+ assert(offset > end);
*length = end - offset;
return i;
}
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PULL v2 0/9] bitmap export over NBD
2018-06-22 12:12 ` Eric Blake
@ 2018-06-22 12:13 ` Peter Maydell
2018-06-22 14:46 ` Eric Blake
0 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2018-06-22 12:13 UTC (permalink / raw)
To: Eric Blake; +Cc: QEMU Developers
On 22 June 2018 at 13:12, Eric Blake <eblake@redhat.com> wrote:
> Uurgh. It's a false positive (the compiler is complaining that the variable
> is uninitialized, which can only happen if the while loop is not executed;
> but the preconditions guarantee the loop executes at least once). The
> assert() that I added was enough to silence gcc 7.3.1 on my Fedora 27
> system, but docker-mingw@fedora is using gcc-8.1.1-1.fc28.x86_64. This
> should silence things (another way to silence would be rewriting while{}
> into do{}while). I'll submit this as a formal patch if I can reproduce the
> problem/fix on docker.
Huh. I had thought this was an old-gcc problem, not a new-gcc
one. Might be worth submitting to the gcc folks as a regression...
thanks
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PULL v2 0/9] bitmap export over NBD
2018-06-22 12:13 ` Peter Maydell
@ 2018-06-22 14:46 ` Eric Blake
0 siblings, 0 replies; 9+ messages in thread
From: Eric Blake @ 2018-06-22 14:46 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers
On 06/22/2018 07:13 AM, Peter Maydell wrote:
> On 22 June 2018 at 13:12, Eric Blake <eblake@redhat.com> wrote:
>> Uurgh. It's a false positive (the compiler is complaining that the variable
>> is uninitialized, which can only happen if the while loop is not executed;
>> but the preconditions guarantee the loop executes at least once). The
>> assert() that I added was enough to silence gcc 7.3.1 on my Fedora 27
>> system, but docker-mingw@fedora is using gcc-8.1.1-1.fc28.x86_64. This
>> should silence things (another way to silence would be rewriting while{}
>> into do{}while). I'll submit this as a formal patch if I can reproduce the
>> problem/fix on docker.
>
> Huh. I had thought this was an old-gcc problem, not a new-gcc
> one. Might be worth submitting to the gcc folks as a regression...
Hmm, I couldn't quickly reproduce it with a direct gcc on Fedora 28; and
I don't know docker well enough to know if the cross-gcc used in
docker-mingw@fedora is a different version than the native gcc-8.1.1
listed in the installed packages list (the patchew output doesn't list
the version of x86_64-w64-mingw32-gcc). Thus, I can't tell if it is a
recent gcc regression or merely an old-gcc issue. But I can confirm
that 'make docker-test-mingw@fedora' reproduced the problem without my
cleanup patch, and that my cleanup patch now in master made that happy
again.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-06-22 14:46 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-21 14:57 [Qemu-devel] [PULL v2 0/9] bitmap export over NBD Eric Blake
2018-06-21 14:57 ` [Qemu-devel] [PULL v2 2/9] nbd/server: Reject 0-length block status request Eric Blake
2018-06-21 14:57 ` [Qemu-devel] [PULL v2 6/9] nbd/server: implement dirty bitmap export Eric Blake
2018-06-21 14:57 ` [Qemu-devel] [PULL v2 9/9] nbd/server: introduce NBD_CMD_CACHE Eric Blake
2018-06-22 9:57 ` [Qemu-devel] [PULL v2 0/9] bitmap export over NBD Peter Maydell
2018-06-22 11:43 ` Peter Maydell
2018-06-22 12:12 ` Eric Blake
2018-06-22 12:13 ` Peter Maydell
2018-06-22 14:46 ` Eric Blake
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.