Re: [Libguestfs] [PATCH v7 12/12] nbd/server: Add FLAG_PAYLOAD support to CMD_BLOCK_STATUS

> +static int > +nbd_co_block_status_payload_read(NBDClient *client, NBDRequest *request, > + Error **errp) > +{ > + int payload_len = request->len; payload_len should be uint64_t > + g_autofree char *buf = NULL; > + size_t count, i, nr_bitmaps; > + uint32_t id; > + otherwise, we may do something unexpected here, when reqeuest->len is too big for int: > + if (payload_len > NBD_MAX_BUFFER_SIZE) { > + error_setg(errp, "len (%" PRIu64 ") is larger than max len (%u)", > + request->len, NBD_MAX_BUFFER_SIZE); > + return -EINVAL; > + }

> + > + assert(client->contexts.exp == client->exp); > + nr_bitmaps = client->exp->nr_export_bitmaps; > + request->contexts = g_new0(NBDMetaContexts, 1); > + request->contexts->exp = client->exp; > + > + if (payload_len % sizeof(uint32_t) || > + payload_len < sizeof(NBDBlockStatusPayload) || > + payload_len > (sizeof(NBDBlockStatusPayload) + > + sizeof(id) * client->contexts.count)) { > + goto skip; > + } [..] > * connection right away, -EAGAIN to indicate we were interrupted and the > @@ -2505,7 +2593,18 @@ static int coroutine_fn nbd_co_receive_request(NBDRequestData *req, > break; > > case NBD_CMD_BLOCK_STATUS: > - request->contexts = &client->contexts; > + if (extended_with_payload) { > + ret = nbd_co_block_status_payload_read(client, request, errp); > + if (ret < 0) { > + return ret; > + } > + /* payload now consumed */ > + check_length = extended_with_payload = false; why set extended_with_payload to false? it's a bit misleading. And you don't do this for WRITE request.

> + payload_len = 0; > + valid_flags |= NBD_CMD_FLAG_PAYLOAD_LEN; > + } else { > + request->contexts = &client->contexts; > + } > valid_flags |= NBD_CMD_FLAG_REQ_ONE; > break; > [..] with payload_len changed to uint64_t, your squash-in applied and s/>/>=/ fixed: Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov(a)yandex-team.ru&gt;