| Message ID | 20230515195343.1915857-14-eblake@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qemu patches for 64-bit NBD extensions | expand |
On 15.05.23 22:53, Eric Blake wrote: > The next commit will add support for the new addition of > NBD_CMD_FLAG_PAYLOAD during NBD_CMD_BLOCK_STATUS, where the client can > request that the server only return a subset of negotiated contexts, > rather than all contexts. To make that task easier, this patch > populates the list of contexts to return on a per-command basis (for > now, identical to the full set of negotiated contexts). > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > include/block/nbd.h | 15 ++++++ > nbd/server.c | 108 +++++++++++++++++++++++--------------------- > 2 files changed, 72 insertions(+), 51 deletions(-) > > diff --git a/include/block/nbd.h b/include/block/nbd.h > index 865bb4ee2e1..6696d61bd59 100644 > --- a/include/block/nbd.h > +++ b/include/block/nbd.h > @@ -60,6 +60,20 @@ typedef enum NBDHeaderStyle { > NBD_HEADER_EXTENDED, /* NBD_OPT_EXTENDED_HEADERS negotiated */ > } NBDHeaderStyle; > > +/* > + * NBDMetaContexts represents a list of meta contexts in use, as > + * selected by NBD_OPT_SET_META_CONTEXT. Also used for > + * NBD_OPT_LIST_META_CONTEXT, and payload filtering in > + * NBD_CMD_BLOCK_STATUS. > + */ > +typedef struct NBDMetaContexts { > + size_t count; /* number of negotiated contexts */ > + bool base_allocation; /* export base:allocation context (block status) */ > + bool allocation_depth; /* export qemu:allocation-depth */ > + size_t nr_bitmaps; /* Length of bitmaps array */ > + bool *bitmaps; /* export qemu:dirty-bitmap:<export bitmap name> */ > +} NBDMetaContexts; > + > /* > * Note: NBDRequest is _NOT_ the same as the network representation of an NBD > * request! > @@ -70,6 +84,7 @@ typedef struct NBDRequest { > uint64_t len; /* Effect length; 32 bit limit without extended headers */ > uint16_t flags; /* NBD_CMD_FLAG_* */ > uint16_t type; /* NBD_CMD_* */ > + NBDMetaContexts contexts; /* Used by NBD_CMD_BLOCK_STATUS */ > } NBDRequest; > > typedef struct NBDSimpleReply { > diff --git a/nbd/server.c b/nbd/server.c > index 6475a76c1f0..db550c82cd2 100644 > --- a/nbd/server.c > +++ b/nbd/server.c > @@ -105,20 +105,6 @@ struct NBDExport { > > static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports); > > -/* NBDExportMetaContexts represents a list of contexts to be exported, > - * as selected by NBD_OPT_SET_META_CONTEXT. Also used for > - * NBD_OPT_LIST_META_CONTEXT. */ > -typedef struct NBDExportMetaContexts { > - NBDExport *exp; > - size_t count; /* number of negotiated contexts */ > - bool base_allocation; /* export base:allocation context (block status) */ > - bool allocation_depth; /* export qemu:allocation-depth */ > - bool *bitmaps; /* > - * export qemu:dirty-bitmap:<export bitmap name>, > - * sized by exp->nr_export_bitmaps > - */ > -} NBDExportMetaContexts; > - > struct NBDClient { > int refcount; > void (*close_fn)(NBDClient *client, bool negotiated); > @@ -144,7 +130,8 @@ struct NBDClient { > uint32_t check_align; /* If non-zero, check for aligned client requests */ > > NBDHeaderStyle header_style; > - NBDExportMetaContexts export_meta; > + NBDExport *context_exp; /* export of last OPT_SET_META_CONTEXT */ > + NBDMetaContexts contexts; /* Negotiated meta contexts */ > > uint32_t opt; /* Current option being negotiated */ > uint32_t optlen; /* remaining length of data in ioc for the option being > @@ -457,8 +444,8 @@ static int nbd_negotiate_handle_list(NBDClient *client, Error **errp) > > static void nbd_check_meta_export(NBDClient *client) > { > - if (client->exp != client->export_meta.exp) { > - client->export_meta.count = 0; > + if (client->exp != client->context_exp) { > + client->contexts.count = 0; > } > } > > @@ -852,7 +839,7 @@ static bool nbd_strshift(const char **str, const char *prefix) > * Handle queries to 'base' namespace. For now, only the base:allocation > * context is available. Return true if @query has been handled. > */ > -static bool nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta, > +static bool nbd_meta_base_query(NBDClient *client, NBDMetaContexts *meta, > const char *query) > { > if (!nbd_strshift(&query, "base:")) { > @@ -872,8 +859,8 @@ static bool nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta, > * and qemu:allocation-depth contexts are available. Return true if @query > * has been handled. > */ > -static bool nbd_meta_qemu_query(NBDClient *client, NBDExportMetaContexts *meta, > - const char *query) > +static bool nbd_meta_qemu_query(NBDClient *client, NBDExport *exp, > + NBDMetaContexts *meta, const char *query) > { > size_t i; > > @@ -884,9 +871,9 @@ static bool nbd_meta_qemu_query(NBDClient *client, NBDExportMetaContexts *meta, > > if (!*query) { > if (client->opt == NBD_OPT_LIST_META_CONTEXT) { > - meta->allocation_depth = meta->exp->allocation_depth; > - if (meta->exp->nr_export_bitmaps) { > - memset(meta->bitmaps, 1, meta->exp->nr_export_bitmaps); > + meta->allocation_depth = exp->allocation_depth; > + if (meta->nr_bitmaps) { > + memset(meta->bitmaps, 1, meta->nr_bitmaps); > } > } > trace_nbd_negotiate_meta_query_parse("empty"); > @@ -895,7 +882,7 @@ static bool nbd_meta_qemu_query(NBDClient *client, NBDExportMetaContexts *meta, > > if (strcmp(query, "allocation-depth") == 0) { > trace_nbd_negotiate_meta_query_parse("allocation-depth"); > - meta->allocation_depth = meta->exp->allocation_depth; > + meta->allocation_depth = exp->allocation_depth; > return true; > } > > @@ -903,17 +890,17 @@ static bool nbd_meta_qemu_query(NBDClient *client, NBDExportMetaContexts *meta, > trace_nbd_negotiate_meta_query_parse("dirty-bitmap:"); > if (!*query) { > if (client->opt == NBD_OPT_LIST_META_CONTEXT && > - meta->exp->nr_export_bitmaps) { > - memset(meta->bitmaps, 1, meta->exp->nr_export_bitmaps); > + exp->nr_export_bitmaps) { > + memset(meta->bitmaps, 1, exp->nr_export_bitmaps); sometimes you change s/meta->exp->nr_export_bitmaps/meta->nr_bitmaps/ and sometimes s/meta->exp->nr_export_bitmaps/exp->nr_export_bitmaps/ > } > trace_nbd_negotiate_meta_query_parse("empty"); > return true; > } > > - for (i = 0; i < meta->exp->nr_export_bitmaps; i++) { > + for (i = 0; i < meta->nr_bitmaps; i++) { > const char *bm_name; > > - bm_name = bdrv_dirty_bitmap_name(meta->exp->export_bitmaps[i]); > + bm_name = bdrv_dirty_bitmap_name(exp->export_bitmaps[i]); I don't like the change: we iterate through exp->export_bitmaps. So seems more correct to use exp->nr_export_bitmaps as a limit. I understand why you want to duplicated nr_export_bitmaps in NBDMetaContexts, but the result doesn't seem better than just use exp->nr_export_bitmaps. Probably, if we want to duplicate nr_export_bitmaps, we should duplicate export_bitmaps as well. But this leads actually to keeping meta->exp as is. Hm, may be just do size_t nr_bitmaps = meta->nr_bitmaps; assert(nr_bitmaps == exp->nr_export_bitmaps) at top of the function? and use nr_bitmaps everywhere. But this again make me doubt that meta.nr_bitmaps is good idea: we have to pass exp together and check meta.nr_bitmaps == exp->nr_export_bitmaps every time. So, I understand, that bool *bitmaps; /* * export qemu:dirty-bitmap:<export bitmap name>, * sized by exp->nr_export_bitmaps */ sounds like not good design - we have an array, sized by field of another object. But, we have a link that that object (meta->exp). But with your way we fall into bad design too: we have a duplicated field, and have to control that they not diverse. And we iterate things in exp, using field of meta. I looked through, seems in all places where we use nr_bitmaps, we also have access to nr_export_bitmaps. So, maybe, drop this nr_bitmaps? And why not to keep meta.exp? With it, meta is self-sufficient object, which fully describe some selected contexts. For per-request filtering, meta.exp just must always point to current export. And anyway would be good to split refactoring of meta structure (if we need it) and new logic of per-command filtering. > if (strcmp(bm_name, query) == 0) { > meta->bitmaps[i] = true; > trace_nbd_negotiate_meta_query_parse(query); > @@ -937,8 +924,8 @@ static bool nbd_meta_qemu_query(NBDClient *client, NBDExportMetaContexts *meta, > * > * 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_negotiate_meta_query(NBDClient *client, > - NBDExportMetaContexts *meta, Error **errp) > +static int nbd_negotiate_meta_query(NBDClient *client, NBDExport *exp, > + NBDMetaContexts *meta, Error **errp) > { > int ret; > g_autofree char *query = NULL; > @@ -965,7 +952,7 @@ static int nbd_negotiate_meta_query(NBDClient *client, > if (nbd_meta_base_query(client, meta, query)) { > return 1; > } > - if (nbd_meta_qemu_query(client, meta, query)) { > + if (nbd_meta_qemu_query(client, exp, meta, query)) { > return 1; > } > > @@ -977,14 +964,15 @@ static int nbd_negotiate_meta_query(NBDClient *client, > * Handle NBD_OPT_LIST_META_CONTEXT and NBD_OPT_SET_META_CONTEXT > * > * Return -errno on I/O error, or 0 if option was completely handled. */ > -static int nbd_negotiate_meta_queries(NBDClient *client, > - NBDExportMetaContexts *meta, Error **errp) > +static int nbd_negotiate_meta_queries(NBDClient *client, Error **errp) > { > int ret; > g_autofree char *export_name = NULL; > /* Mark unused to work around https://bugs.llvm.org/show_bug.cgi?id=3888 */ > g_autofree G_GNUC_UNUSED bool *bitmaps = NULL; > - NBDExportMetaContexts local_meta = {0}; > + NBDMetaContexts local_meta = {0}; > + NBDMetaContexts *meta; > + NBDExport *exp; > uint32_t nb_queries; > size_t i; > size_t count = 0; > @@ -1000,6 +988,9 @@ static int nbd_negotiate_meta_queries(NBDClient *client, > if (client->opt == NBD_OPT_LIST_META_CONTEXT) { > /* Only change the caller's meta on SET. */ > meta = &local_meta; > + } else { > + meta = &client->contexts; > + client->context_exp = NULL; > } > > g_free(meta->bitmaps); > @@ -1010,14 +1001,15 @@ static int nbd_negotiate_meta_queries(NBDClient *client, > return ret; > } > > - meta->exp = nbd_export_find(export_name); > - if (meta->exp == NULL) { > + exp = nbd_export_find(export_name); > + if (exp == NULL) { > g_autofree char *sane_name = nbd_sanitize_name(export_name); > > return nbd_opt_drop(client, NBD_REP_ERR_UNKNOWN, errp, > "export '%s' not present", sane_name); > } > - meta->bitmaps = g_new0(bool, meta->exp->nr_export_bitmaps); > + meta->nr_bitmaps = exp->nr_export_bitmaps; > + meta->bitmaps = g_new0(bool, exp->nr_export_bitmaps); > if (client->opt == NBD_OPT_LIST_META_CONTEXT) { > bitmaps = meta->bitmaps; > } > @@ -1033,13 +1025,13 @@ 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->allocation_depth = meta->exp->allocation_depth; > - if (meta->exp->nr_export_bitmaps) { > - memset(meta->bitmaps, 1, meta->exp->nr_export_bitmaps); > + meta->allocation_depth = exp->allocation_depth; > + if (exp->nr_export_bitmaps) { > + memset(meta->bitmaps, 1, meta->nr_bitmaps); > } > } else { > for (i = 0; i < nb_queries; ++i) { > - ret = nbd_negotiate_meta_query(client, meta, errp); > + ret = nbd_negotiate_meta_query(client, exp, meta, errp); > if (ret <= 0) { > return ret; > } > @@ -1066,7 +1058,7 @@ static int nbd_negotiate_meta_queries(NBDClient *client, > count++; > } > > - for (i = 0; i < meta->exp->nr_export_bitmaps; i++) { > + for (i = 0; i < meta->nr_bitmaps; i++) { > const char *bm_name; > g_autofree char *context = NULL; > > @@ -1074,7 +1066,7 @@ static int nbd_negotiate_meta_queries(NBDClient *client, > continue; > } > > - bm_name = bdrv_dirty_bitmap_name(meta->exp->export_bitmaps[i]); > + bm_name = bdrv_dirty_bitmap_name(exp->export_bitmaps[i]); > context = g_strdup_printf("qemu:dirty-bitmap:%s", bm_name); > > ret = nbd_negotiate_send_meta_context(client, context, > @@ -1089,6 +1081,9 @@ static int nbd_negotiate_meta_queries(NBDClient *client, > ret = nbd_negotiate_send_rep(client, NBD_REP_ACK, errp); > if (ret == 0) { > meta->count = count; > + if (client->opt == NBD_OPT_SET_META_CONTEXT) { > + client->context_exp = exp; > + } > } > > return ret; > @@ -1282,8 +1277,7 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp) > > case NBD_OPT_LIST_META_CONTEXT: > case NBD_OPT_SET_META_CONTEXT: > - ret = nbd_negotiate_meta_queries(client, &client->export_meta, > - errp); > + ret = nbd_negotiate_meta_queries(client, errp); > break; > > case NBD_OPT_EXTENDED_HEADERS: > @@ -1514,7 +1508,7 @@ void nbd_client_put(NBDClient *client) > QTAILQ_REMOVE(&client->exp->clients, client, next); > blk_exp_unref(&client->exp->common); > } > - g_free(client->export_meta.bitmaps); > + g_free(client->contexts.bitmaps); > g_free(client); > } > } > @@ -2489,6 +2483,8 @@ static int coroutine_fn nbd_co_receive_request(NBDRequestData *req, NBDRequest * > return -ENOMEM; > } > } > + } else if (request->type == NBD_CMD_BLOCK_STATUS) { > + request->contexts = client->contexts; > } > > if (payload_len) { > @@ -2715,11 +2711,11 @@ static coroutine_fn int nbd_handle_request(NBDClient *client, > } > assert(client->header_style >= NBD_HEADER_EXTENDED || > request->len <= UINT32_MAX); > - if (client->export_meta.count) { > + if (request->contexts.count) { > bool dont_fragment = request->flags & NBD_CMD_FLAG_REQ_ONE; > - int contexts_remaining = client->export_meta.count; > + int contexts_remaining = request->contexts.count; > > - if (client->export_meta.base_allocation) { > + if (request->contexts.base_allocation) { > ret = nbd_co_send_block_status(client, request, > exp->common.blk, > request->from, > @@ -2732,7 +2728,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client, > } > } > > - if (client->export_meta.allocation_depth) { > + if (request->contexts.allocation_depth) { > ret = nbd_co_send_block_status(client, request, > exp->common.blk, > request->from, request->len, > @@ -2745,8 +2741,10 @@ static coroutine_fn int nbd_handle_request(NBDClient *client, > } > } > > + assert(request->contexts.nr_bitmaps == > + client->exp->nr_export_bitmaps); > for (i = 0; i < client->exp->nr_export_bitmaps; i++) { > - if (!client->export_meta.bitmaps[i]) { > + if (!request->contexts.bitmaps[i]) { > continue; > } > ret = nbd_co_send_bitmap(client, request, > @@ -2762,6 +2760,10 @@ static coroutine_fn int nbd_handle_request(NBDClient *client, > assert(!contexts_remaining); > > return 0; > + } else if (client->contexts.count) { > + return nbd_send_generic_reply(client, request, -EINVAL, > + "CMD_BLOCK_STATUS payload not valid", > + errp); > } else { > return nbd_send_generic_reply(client, request, -EINVAL, > "CMD_BLOCK_STATUS not negotiated", > @@ -2840,6 +2842,10 @@ static coroutine_fn void nbd_trip(void *opaque) > } else { > ret = nbd_handle_request(client, &request, req->data, &local_err); > } > + if (request.type == NBD_CMD_BLOCK_STATUS && > + request.contexts.bitmaps != client->contexts.bitmaps) { > + g_free(request.contexts.bitmaps); > + } > if (ret < 0) { > error_prepend(&local_err, "Failed to send reply: "); > goto disconnect;
diff --git a/include/block/nbd.h b/include/block/nbd.h index 865bb4ee2e1..6696d61bd59 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -60,6 +60,20 @@ typedef enum NBDHeaderStyle { NBD_HEADER_EXTENDED, /* NBD_OPT_EXTENDED_HEADERS negotiated */ } NBDHeaderStyle; +/* + * NBDMetaContexts represents a list of meta contexts in use, as + * selected by NBD_OPT_SET_META_CONTEXT. Also used for + * NBD_OPT_LIST_META_CONTEXT, and payload filtering in + * NBD_CMD_BLOCK_STATUS. + */ +typedef struct NBDMetaContexts { + size_t count; /* number of negotiated contexts */ + bool base_allocation; /* export base:allocation context (block status) */ + bool allocation_depth; /* export qemu:allocation-depth */ + size_t nr_bitmaps; /* Length of bitmaps array */ + bool *bitmaps; /* export qemu:dirty-bitmap:<export bitmap name> */ +} NBDMetaContexts; + /* * Note: NBDRequest is _NOT_ the same as the network representation of an NBD * request! @@ -70,6 +84,7 @@ typedef struct NBDRequest { uint64_t len; /* Effect length; 32 bit limit without extended headers */ uint16_t flags; /* NBD_CMD_FLAG_* */ uint16_t type; /* NBD_CMD_* */ + NBDMetaContexts contexts; /* Used by NBD_CMD_BLOCK_STATUS */ } NBDRequest; typedef struct NBDSimpleReply { diff --git a/nbd/server.c b/nbd/server.c index 6475a76c1f0..db550c82cd2 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -105,20 +105,6 @@ struct NBDExport { static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports); -/* NBDExportMetaContexts represents a list of contexts to be exported, - * as selected by NBD_OPT_SET_META_CONTEXT. Also used for - * NBD_OPT_LIST_META_CONTEXT. */ -typedef struct NBDExportMetaContexts { - NBDExport *exp; - size_t count; /* number of negotiated contexts */ - bool base_allocation; /* export base:allocation context (block status) */ - bool allocation_depth; /* export qemu:allocation-depth */ - bool *bitmaps; /* - * export qemu:dirty-bitmap:<export bitmap name>, - * sized by exp->nr_export_bitmaps - */ -} NBDExportMetaContexts; - struct NBDClient { int refcount; void (*close_fn)(NBDClient *client, bool negotiated); @@ -144,7 +130,8 @@ struct NBDClient { uint32_t check_align; /* If non-zero, check for aligned client requests */ NBDHeaderStyle header_style; - NBDExportMetaContexts export_meta; + NBDExport *context_exp; /* export of last OPT_SET_META_CONTEXT */ + NBDMetaContexts contexts; /* Negotiated meta contexts */ uint32_t opt; /* Current option being negotiated */ uint32_t optlen; /* remaining length of data in ioc for the option being @@ -457,8 +444,8 @@ static int nbd_negotiate_handle_list(NBDClient *client, Error **errp) static void nbd_check_meta_export(NBDClient *client) { - if (client->exp != client->export_meta.exp) { - client->export_meta.count = 0; + if (client->exp != client->context_exp) { + client->contexts.count = 0; } } @@ -852,7 +839,7 @@ static bool nbd_strshift(const char **str, const char *prefix) * Handle queries to 'base' namespace. For now, only the base:allocation * context is available. Return true if @query has been handled. */ -static bool nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta, +static bool nbd_meta_base_query(NBDClient *client, NBDMetaContexts *meta, const char *query) { if (!nbd_strshift(&query, "base:")) { @@ -872,8 +859,8 @@ static bool nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta, * and qemu:allocation-depth contexts are available. Return true if @query * has been handled. */ -static bool nbd_meta_qemu_query(NBDClient *client, NBDExportMetaContexts *meta, - const char *query) +static bool nbd_meta_qemu_query(NBDClient *client, NBDExport *exp, + NBDMetaContexts *meta, const char *query) { size_t i; @@ -884,9 +871,9 @@ static bool nbd_meta_qemu_query(NBDClient *client, NBDExportMetaContexts *meta, if (!*query) { if (client->opt == NBD_OPT_LIST_META_CONTEXT) { - meta->allocation_depth = meta->exp->allocation_depth; - if (meta->exp->nr_export_bitmaps) { - memset(meta->bitmaps, 1, meta->exp->nr_export_bitmaps); + meta->allocation_depth = exp->allocation_depth; + if (meta->nr_bitmaps) { + memset(meta->bitmaps, 1, meta->nr_bitmaps); } } trace_nbd_negotiate_meta_query_parse("empty"); @@ -895,7 +882,7 @@ static bool nbd_meta_qemu_query(NBDClient *client, NBDExportMetaContexts *meta, if (strcmp(query, "allocation-depth") == 0) { trace_nbd_negotiate_meta_query_parse("allocation-depth"); - meta->allocation_depth = meta->exp->allocation_depth; + meta->allocation_depth = exp->allocation_depth; return true; } @@ -903,17 +890,17 @@ static bool nbd_meta_qemu_query(NBDClient *client, NBDExportMetaContexts *meta, trace_nbd_negotiate_meta_query_parse("dirty-bitmap:"); if (!*query) { if (client->opt == NBD_OPT_LIST_META_CONTEXT && - meta->exp->nr_export_bitmaps) { - memset(meta->bitmaps, 1, meta->exp->nr_export_bitmaps); + exp->nr_export_bitmaps) { + memset(meta->bitmaps, 1, exp->nr_export_bitmaps); } trace_nbd_negotiate_meta_query_parse("empty"); return true; } - for (i = 0; i < meta->exp->nr_export_bitmaps; i++) { + for (i = 0; i < meta->nr_bitmaps; i++) { const char *bm_name; - bm_name = bdrv_dirty_bitmap_name(meta->exp->export_bitmaps[i]); + bm_name = bdrv_dirty_bitmap_name(exp->export_bitmaps[i]); if (strcmp(bm_name, query) == 0) { meta->bitmaps[i] = true; trace_nbd_negotiate_meta_query_parse(query); @@ -937,8 +924,8 @@ static bool nbd_meta_qemu_query(NBDClient *client, NBDExportMetaContexts *meta, * * 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_negotiate_meta_query(NBDClient *client, - NBDExportMetaContexts *meta, Error **errp) +static int nbd_negotiate_meta_query(NBDClient *client, NBDExport *exp, + NBDMetaContexts *meta, Error **errp) { int ret; g_autofree char *query = NULL; @@ -965,7 +952,7 @@ static int nbd_negotiate_meta_query(NBDClient *client, if (nbd_meta_base_query(client, meta, query)) { return 1; } - if (nbd_meta_qemu_query(client, meta, query)) { + if (nbd_meta_qemu_query(client, exp, meta, query)) { return 1; } @@ -977,14 +964,15 @@ static int nbd_negotiate_meta_query(NBDClient *client, * Handle NBD_OPT_LIST_META_CONTEXT and NBD_OPT_SET_META_CONTEXT * * Return -errno on I/O error, or 0 if option was completely handled. */ -static int nbd_negotiate_meta_queries(NBDClient *client, - NBDExportMetaContexts *meta, Error **errp) +static int nbd_negotiate_meta_queries(NBDClient *client, Error **errp) { int ret; g_autofree char *export_name = NULL; /* Mark unused to work around https://bugs.llvm.org/show_bug.cgi?id=3888 */ g_autofree G_GNUC_UNUSED bool *bitmaps = NULL; - NBDExportMetaContexts local_meta = {0}; + NBDMetaContexts local_meta = {0}; + NBDMetaContexts *meta; + NBDExport *exp; uint32_t nb_queries; size_t i; size_t count = 0; @@ -1000,6 +988,9 @@ static int nbd_negotiate_meta_queries(NBDClient *client, if (client->opt == NBD_OPT_LIST_META_CONTEXT) { /* Only change the caller's meta on SET. */ meta = &local_meta; + } else { + meta = &client->contexts; + client->context_exp = NULL; } g_free(meta->bitmaps); @@ -1010,14 +1001,15 @@ static int nbd_negotiate_meta_queries(NBDClient *client, return ret; } - meta->exp = nbd_export_find(export_name); - if (meta->exp == NULL) { + exp = nbd_export_find(export_name); + if (exp == NULL) { g_autofree char *sane_name = nbd_sanitize_name(export_name); return nbd_opt_drop(client, NBD_REP_ERR_UNKNOWN, errp, "export '%s' not present", sane_name); } - meta->bitmaps = g_new0(bool, meta->exp->nr_export_bitmaps); + meta->nr_bitmaps = exp->nr_export_bitmaps; + meta->bitmaps = g_new0(bool, exp->nr_export_bitmaps); if (client->opt == NBD_OPT_LIST_META_CONTEXT) { bitmaps = meta->bitmaps; } @@ -1033,13 +1025,13 @@ 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->allocation_depth = meta->exp->allocation_depth; - if (meta->exp->nr_export_bitmaps) { - memset(meta->bitmaps, 1, meta->exp->nr_export_bitmaps); + meta->allocation_depth = exp->allocation_depth; + if (exp->nr_export_bitmaps) { + memset(meta->bitmaps, 1, meta->nr_bitmaps); } } else { for (i = 0; i < nb_queries; ++i) { - ret = nbd_negotiate_meta_query(client, meta, errp); + ret = nbd_negotiate_meta_query(client, exp, meta, errp); if (ret <= 0) { return ret; } @@ -1066,7 +1058,7 @@ static int nbd_negotiate_meta_queries(NBDClient *client, count++; } - for (i = 0; i < meta->exp->nr_export_bitmaps; i++) { + for (i = 0; i < meta->nr_bitmaps; i++) { const char *bm_name; g_autofree char *context = NULL; @@ -1074,7 +1066,7 @@ static int nbd_negotiate_meta_queries(NBDClient *client, continue; } - bm_name = bdrv_dirty_bitmap_name(meta->exp->export_bitmaps[i]); + bm_name = bdrv_dirty_bitmap_name(exp->export_bitmaps[i]); context = g_strdup_printf("qemu:dirty-bitmap:%s", bm_name); ret = nbd_negotiate_send_meta_context(client, context, @@ -1089,6 +1081,9 @@ static int nbd_negotiate_meta_queries(NBDClient *client, ret = nbd_negotiate_send_rep(client, NBD_REP_ACK, errp); if (ret == 0) { meta->count = count; + if (client->opt == NBD_OPT_SET_META_CONTEXT) { + client->context_exp = exp; + } } return ret; @@ -1282,8 +1277,7 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp) case NBD_OPT_LIST_META_CONTEXT: case NBD_OPT_SET_META_CONTEXT: - ret = nbd_negotiate_meta_queries(client, &client->export_meta, - errp); + ret = nbd_negotiate_meta_queries(client, errp); break; case NBD_OPT_EXTENDED_HEADERS: @@ -1514,7 +1508,7 @@ void nbd_client_put(NBDClient *client) QTAILQ_REMOVE(&client->exp->clients, client, next); blk_exp_unref(&client->exp->common); } - g_free(client->export_meta.bitmaps); + g_free(client->contexts.bitmaps); g_free(client); } } @@ -2489,6 +2483,8 @@ static int coroutine_fn nbd_co_receive_request(NBDRequestData *req, NBDRequest * return -ENOMEM; } } + } else if (request->type == NBD_CMD_BLOCK_STATUS) { + request->contexts = client->contexts; } if (payload_len) { @@ -2715,11 +2711,11 @@ static coroutine_fn int nbd_handle_request(NBDClient *client, } assert(client->header_style >= NBD_HEADER_EXTENDED || request->len <= UINT32_MAX); - if (client->export_meta.count) { + if (request->contexts.count) { bool dont_fragment = request->flags & NBD_CMD_FLAG_REQ_ONE; - int contexts_remaining = client->export_meta.count; + int contexts_remaining = request->contexts.count; - if (client->export_meta.base_allocation) { + if (request->contexts.base_allocation) { ret = nbd_co_send_block_status(client, request, exp->common.blk, request->from, @@ -2732,7 +2728,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client, } } - if (client->export_meta.allocation_depth) { + if (request->contexts.allocation_depth) { ret = nbd_co_send_block_status(client, request, exp->common.blk, request->from, request->len, @@ -2745,8 +2741,10 @@ static coroutine_fn int nbd_handle_request(NBDClient *client, } } + assert(request->contexts.nr_bitmaps == + client->exp->nr_export_bitmaps); for (i = 0; i < client->exp->nr_export_bitmaps; i++) { - if (!client->export_meta.bitmaps[i]) { + if (!request->contexts.bitmaps[i]) { continue; } ret = nbd_co_send_bitmap(client, request, @@ -2762,6 +2760,10 @@ static coroutine_fn int nbd_handle_request(NBDClient *client, assert(!contexts_remaining); return 0; + } else if (client->contexts.count) { + return nbd_send_generic_reply(client, request, -EINVAL, + "CMD_BLOCK_STATUS payload not valid", + errp); } else { return nbd_send_generic_reply(client, request, -EINVAL, "CMD_BLOCK_STATUS not negotiated", @@ -2840,6 +2842,10 @@ static coroutine_fn void nbd_trip(void *opaque) } else { ret = nbd_handle_request(client, &request, req->data, &local_err); } + if (request.type == NBD_CMD_BLOCK_STATUS && + request.contexts.bitmaps != client->contexts.bitmaps) { + g_free(request.contexts.bitmaps); + } if (ret < 0) { error_prepend(&local_err, "Failed to send reply: "); goto disconnect;
The next commit will add support for the new addition of NBD_CMD_FLAG_PAYLOAD during NBD_CMD_BLOCK_STATUS, where the client can request that the server only return a subset of negotiated contexts, rather than all contexts. To make that task easier, this patch populates the list of contexts to return on a per-command basis (for now, identical to the full set of negotiated contexts). Signed-off-by: Eric Blake <eblake@redhat.com> --- include/block/nbd.h | 15 ++++++ nbd/server.c | 108 +++++++++++++++++++++++--------------------- 2 files changed, 72 insertions(+), 51 deletions(-)