Re: [Libguestfs] [libnbd PATCH v2 07/12] api: Make nbd_opt_list_meta_context stateless

On Wed, Aug 31, 2022 at 09:39:23AM -0500, Eric Blake wrote: > Since NBD_OPT_LIST_META_CONTEXTS is stateless from the server's point > of view, there is no reason to make it clear state on the client's > side. Since we do not otherwise modify h->meta_contexts while > processing nbd_opt_list_meta_contexts(), we also should not wipe it; > similarly, while clearing h->exportsize makes sense if we know we are > going to attempt NBD_OPT_INFO or NBD_OPT_GO, it should not be > attempted merely because we are doing NBD_OPT_LIST_META_CONTEXTS. > > Testsuite coverage for this is in the next patch, for ease of review > (that is, this is one case where it is easy to swap the patch order to > see a failure fixed by this patch). > --- > generator/states-newstyle-opt-meta-context.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Given the review comments on 3/12, I'm thinking of squashing in: diff --git c/generator/states-newstyle-opt-go.c w/generator/states-newstyle-opt-go.c index 1ca5f099..9b05bec1 100644 --- c/generator/states-newstyle-opt-go.c +++ w/generator/states-newstyle-opt-go.c @@ -22,6 +22,7 @@ STATE_MACHINE { NEWSTYLE.OPT_GO.START: uint16_t nrinfos = 0; + nbd_internal_reset_size_and_flags (h); if (h->request_block_size) nrinfos++; if (h->full_info) diff --git c/generator/states-newstyle-opt-meta-context.c w/generator/states-newstyle-opt-meta-context.c index 3e66b106..6149d61d 100644 --- c/generator/states-newstyle-opt-meta-context.c +++ w/generator/states-newstyle-opt-meta-context.c @@ -35,12 +35,12 @@ STATE_MACHINE { * nbd_opt_list_meta_context() * -> unconditionally use LIST, next state NEGOTIATING * - * For now, we start by unconditionally clearing h->exportsize and friends. * If SET is conditional, we skip it if h->request_meta is false, if * structured replies were not negotiated, or if no contexts to request. * If OPT_GO is later successful, it populates h->exportsize and friends, * and also sets h->meta_valid if h->request_meta but we skipped SET here. - * SET then manipulates h->meta_contexts, and sets h->meta_valid on success. + * SET then manipulates h->meta_contexts, and sets h->meta_valid on + * success, while LIST is stateless. * There is a callback if and only if the command is unconditional. */ assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE); @@ -52,7 +52,6 @@ STATE_MACHINE { else { assert (CALLBACK_IS_NULL (h->opt_cb.fn.context)); opt = NBD_OPT_SET_META_CONTEXT; - nbd_internal_reset_size_and_flags (h); if (h->request_meta) { for (i = 0; i < h->meta_contexts.len; ++i) free (h->meta_contexts.ptr[i].name);