{"id":831212,"url":"http://patchwork.ozlabs.org/api/1.2/patches/831212/?format=json","web_url":"http://patchwork.ozlabs.org/project/qemu-devel/patch/20171027104037.8319-13-eblake@redhat.com/","project":{"id":14,"url":"http://patchwork.ozlabs.org/api/1.2/projects/14/?format=json","name":"QEMU Development","link_name":"qemu-devel","list_id":"qemu-devel.nongnu.org","list_email":"qemu-devel@nongnu.org","web_url":"","scm_url":"","webscm_url":"","list_archive_url":"","list_archive_url_format":"","commit_url_format":""},"msgid":"<20171027104037.8319-13-eblake@redhat.com>","list_archive_url":null,"date":"2017-10-27T10:40:37","name":"[v6,12/12] nbd: Minimal structured read for client","commit_ref":null,"pull_url":null,"state":"new","archived":false,"hash":"39493d039827579428eb218dcaf519a6ef486370","submitter":{"id":6591,"url":"http://patchwork.ozlabs.org/api/1.2/people/6591/?format=json","name":"Eric Blake","email":"eblake@redhat.com"},"delegate":null,"mbox":"http://patchwork.ozlabs.org/project/qemu-devel/patch/20171027104037.8319-13-eblake@redhat.com/mbox/","series":[{"id":10552,"url":"http://patchwork.ozlabs.org/api/1.2/series/10552/?format=json","web_url":"http://patchwork.ozlabs.org/project/qemu-devel/list/?series=10552","date":"2017-10-27T10:40:27","name":"nbd minimal structured read","version":6,"mbox":"http://patchwork.ozlabs.org/series/10552/mbox/"}],"comments":"http://patchwork.ozlabs.org/api/patches/831212/comments/","check":"pending","checks":"http://patchwork.ozlabs.org/api/patches/831212/checks/","tags":{},"related":[],"headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=eblake@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yNght3BhXz9sNx\n\tfor ;\n\tFri, 27 Oct 2017 21:53:34 +1100 (AEDT)","from localhost ([::1]:56596 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1e82Ga-0001bN-EP\n\tfor incoming@patchwork.ozlabs.org; Fri, 27 Oct 2017 06:53:32 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:57581)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1e824r-0001gj-2u\n\tfor qemu-devel@nongnu.org; Fri, 27 Oct 2017 06:41:28 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1e824j-0005rc-S4\n\tfor qemu-devel@nongnu.org; Fri, 27 Oct 2017 06:41:25 -0400","from mx1.redhat.com ([209.132.183.28]:50668)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1e824d-0005o7-OA; Fri, 27 Oct 2017 06:41:12 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 8379280468;\n\tFri, 27 Oct 2017 10:41:10 +0000 (UTC)","from red.redhat.com (ovpn-120-166.rdu2.redhat.com [10.10.120.166])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id BE32E5C881;\n\tFri, 27 Oct 2017 10:41:08 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 8379280468","From":"Eric Blake ","To":"qemu-devel@nongnu.org","Date":"Fri, 27 Oct 2017 12:40:37 +0200","Message-Id":"<20171027104037.8319-13-eblake@redhat.com>","In-Reply-To":"<20171027104037.8319-1-eblake@redhat.com>","References":"<20171027104037.8319-1-eblake@redhat.com>","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.28]);\n\tFri, 27 Oct 2017 10:41:10 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"[Qemu-devel] [PATCH v6 12/12] nbd: Minimal structured read for\n\tclient","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Kevin Wolf , pbonzini@redhat.com,\n\tvsementsov@virtuozzo.com, qemu-block@nongnu.org,\n\tMax Reitz ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"},"content":"From: Vladimir Sementsov-Ogievskiy \n\nMinimal implementation: for structured error only error_report error\nmessage.\n\nNote that test 83 is now more verbose, because the implementation\nprints more warnings about unexpected communication errors; perhaps\nfuture patches should tone things down by using trace messages\ninstead of traces, but the common case of successful communication\nis no noisier than before.\n\nSigned-off-by: Vladimir Sementsov-Ogievskiy \nSigned-off-by: Eric Blake \n\n---\nv6: tweak overflow check [Vladimir], fix reads to use absolute offset\nfrom server by tracking original offset, fix talking to old-style server,\ntweak iotest 83 output to account for new verbosity\nv5: fix payload_advance[32,64], return correct negative error on\nstructured error, rearrange size checks to not be vulnerable to\noverflow, simplify payload to use g_new instead of qemu_memalign,\ndon't set errp when returning 0, validate that error message\nlength is sane\n---\n block/nbd-client.h | 1 +\n include/block/nbd.h | 12 ++\n nbd/nbd-internal.h | 1 -\n block/nbd-client.c | 490 ++++++++++++++++++++++++++++++++++++++++++---\n nbd/client.c | 12 ++\n tests/qemu-iotests/083.out | 15 ++\n 6 files changed, 498 insertions(+), 33 deletions(-)","diff":"diff --git a/block/nbd-client.h b/block/nbd-client.h\nindex b435754b82..612c4c21a0 100644\n--- a/block/nbd-client.h\n+++ b/block/nbd-client.h\n@@ -19,6 +19,7 @@\n\n typedef struct {\n Coroutine *coroutine;\n+ uint64_t offset; /* original offset of the request */\n bool receiving; /* waiting for read_reply_co? */\n } NBDClientRequest;\n\ndiff --git a/include/block/nbd.h b/include/block/nbd.h\nindex da6e305dd5..92d1723d7c 100644\n--- a/include/block/nbd.h\n+++ b/include/block/nbd.h\n@@ -197,6 +197,11 @@ enum {\n #define NBD_REPLY_TYPE_ERROR NBD_REPLY_ERR(1)\n #define NBD_REPLY_TYPE_ERROR_OFFSET NBD_REPLY_ERR(2)\n\n+static inline bool nbd_reply_type_is_error(int type)\n+{\n+ return type & (1 << 15);\n+}\n+\n /* NBD errors are based on errno numbers, so there is a 1:1 mapping,\n * but only a limited set of errno values is specified in the protocol.\n * Everything else is squashed to EINVAL.\n@@ -214,6 +219,11 @@ enum {\n struct NBDExportInfo {\n /* Set by client before nbd_receive_negotiate() */\n bool request_sizes;\n+\n+ /* In-out fields, set by client before nbd_receive_negotiate() and\n+ * updated by server results during nbd_receive_negotiate() */\n+ bool structured_reply;\n+\n /* Set by server results during nbd_receive_negotiate() */\n uint64_t size;\n uint16_t flags;\n@@ -284,4 +294,6 @@ static inline bool nbd_reply_is_structured(NBDReply *reply)\n return reply->magic == NBD_STRUCTURED_REPLY_MAGIC;\n }\n\n+const char *nbd_reply_type_lookup(uint16_t type);\n+\n #endif\ndiff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h\nindex b64eb1cc9b..eeff78d3c9 100644\n--- a/nbd/nbd-internal.h\n+++ b/nbd/nbd-internal.h\n@@ -104,7 +104,6 @@ const char *nbd_opt_lookup(uint32_t opt);\n const char *nbd_rep_lookup(uint32_t rep);\n const char *nbd_info_lookup(uint16_t info);\n const char *nbd_cmd_lookup(uint16_t info);\n-const char *nbd_reply_type_lookup(uint16_t type);\n const char *nbd_err_lookup(int err);\n\n int nbd_drop(QIOChannel *ioc, size_t size, Error **errp);\ndiff --git a/block/nbd-client.c b/block/nbd-client.c\nindex 58493b7ac4..b44d4d4a01 100644\n--- a/block/nbd-client.c\n+++ b/block/nbd-client.c\n@@ -93,7 +93,7 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)\n if (i >= MAX_NBD_REQUESTS ||\n !s->requests[i].coroutine ||\n !s->requests[i].receiving ||\n- nbd_reply_is_structured(&s->reply))\n+ (nbd_reply_is_structured(&s->reply) && !s->info.structured_reply))\n {\n break;\n }\n@@ -141,6 +141,7 @@ static int nbd_co_send_request(BlockDriverState *bs,\n assert(i < MAX_NBD_REQUESTS);\n\n s->requests[i].coroutine = qemu_coroutine_self();\n+ s->requests[i].offset = request->from;\n s->requests[i].receiving = false;\n\n request->handle = INDEX_TO_HANDLE(s, i);\n@@ -181,75 +182,489 @@ err:\n return rc;\n }\n\n-static int nbd_co_receive_reply(NBDClientSession *s,\n- uint64_t handle,\n- QEMUIOVector *qiov)\n+static inline uint16_t payload_advance16(uint8_t **payload)\n+{\n+ *payload += 2;\n+ return lduw_be_p(*payload - 2);\n+}\n+\n+static inline uint32_t payload_advance32(uint8_t **payload)\n+{\n+ *payload += 4;\n+ return ldl_be_p(*payload - 4);\n+}\n+\n+static inline uint64_t payload_advance64(uint8_t **payload)\n+{\n+ *payload += 8;\n+ return ldq_be_p(*payload - 8);\n+}\n+\n+static int nbd_parse_offset_hole_payload(NBDStructuredReplyChunk *chunk,\n+ uint8_t *payload, uint64_t orig_offset,\n+ QEMUIOVector *qiov, Error **errp)\n+{\n+ uint64_t offset;\n+ uint32_t hole_size;\n+\n+ if (chunk->length != sizeof(offset) + sizeof(hole_size)) {\n+ error_setg(errp, \"Protocol error: invalid payload for \"\n+ \"NBD_REPLY_TYPE_OFFSET_HOLE\");\n+ return -EINVAL;\n+ }\n+\n+ offset = payload_advance64(&payload);\n+ hole_size = payload_advance32(&payload);\n+\n+ if (offset < orig_offset || hole_size > qiov->size ||\n+ offset > orig_offset + qiov->size - hole_size) {\n+ error_setg(errp, \"Protocol error: server sent chunk exceeding requested\"\n+ \" region\");\n+ return -EINVAL;\n+ }\n+\n+ qemu_iovec_memset(qiov, offset - orig_offset, 0, hole_size);\n+\n+ return 0;\n+}\n+\n+/* nbd_parse_error_payload\n+ * on success @errp contains message describing nbd error reply\n+ */\n+static int nbd_parse_error_payload(NBDStructuredReplyChunk *chunk,\n+ uint8_t *payload, int *request_ret,\n+ Error **errp)\n+{\n+ uint32_t error;\n+ uint16_t message_size;\n+\n+ assert(chunk->type & (1 << 15));\n+\n+ if (chunk->length < sizeof(error) + sizeof(message_size)) {\n+ error_setg(errp,\n+ \"Protocol error: invalid payload for structured error\");\n+ return -EINVAL;\n+ }\n+\n+ error = nbd_errno_to_system_errno(payload_advance32(&payload));\n+ if (error == 0) {\n+ error_setg(errp, \"Protocol error: server sent structured error chunk\"\n+ \"with error = 0\");\n+ return -EINVAL;\n+ }\n+\n+ *request_ret = -error;\n+ message_size = payload_advance16(&payload);\n+\n+ if (message_size > chunk->length - sizeof(error) - sizeof(message_size)) {\n+ error_setg(errp, \"Protocol error: server sent structured error chunk\"\n+ \"with incorrect message size\");\n+ return -EINVAL;\n+ }\n+\n+ /* TODO: Add a trace point to mention the server complaint */\n+\n+ /* TODO handle ERROR_OFFSET */\n+\n+ return 0;\n+}\n+\n+static int nbd_co_receive_offset_data_payload(NBDClientSession *s,\n+ uint64_t orig_offset,\n+ QEMUIOVector *qiov, Error **errp)\n+{\n+ QEMUIOVector sub_qiov;\n+ uint64_t offset;\n+ size_t data_size;\n+ int ret;\n+ NBDStructuredReplyChunk *chunk = &s->reply.structured;\n+\n+ assert(nbd_reply_is_structured(&s->reply));\n+\n+ if (chunk->length < sizeof(offset)) {\n+ error_setg(errp, \"Protocol error: invalid payload for \"\n+ \"NBD_REPLY_TYPE_OFFSET_DATA\");\n+ return -EINVAL;\n+ }\n+\n+ if (nbd_read(s->ioc, &offset, sizeof(offset), errp) < 0) {\n+ return -EIO;\n+ }\n+ be64_to_cpus(&offset);\n+\n+ data_size = chunk->length - sizeof(offset);\n+ if (offset < orig_offset || data_size > qiov->size ||\n+ offset > orig_offset + qiov->size - data_size) {\n+ error_setg(errp, \"Protocol error: server sent chunk exceeding requested\"\n+ \" region\");\n+ return -EINVAL;\n+ }\n+\n+ qemu_iovec_init(&sub_qiov, qiov->niov);\n+ qemu_iovec_concat(&sub_qiov, qiov, offset - orig_offset, data_size);\n+ ret = qio_channel_readv_all(s->ioc, sub_qiov.iov, sub_qiov.niov, errp);\n+ qemu_iovec_destroy(&sub_qiov);\n+\n+ return ret < 0 ? -EIO : 0;\n+}\n+\n+#define NBD_MAX_MALLOC_PAYLOAD 1000\n+/* nbd_co_receive_structured_payload\n+ */\n+static coroutine_fn int nbd_co_receive_structured_payload(\n+ NBDClientSession *s, void **payload, Error **errp)\n+{\n+ int ret;\n+ uint32_t len;\n+\n+ assert(nbd_reply_is_structured(&s->reply));\n+\n+ len = s->reply.structured.length;\n+\n+ if (len == 0) {\n+ return 0;\n+ }\n+\n+ if (payload == NULL) {\n+ error_setg(errp, \"Unexpected structured payload\");\n+ return -EINVAL;\n+ }\n+\n+ if (len > NBD_MAX_MALLOC_PAYLOAD) {\n+ error_setg(errp, \"Payload too large\");\n+ return -EINVAL;\n+ }\n+\n+ *payload = g_new(char, len);\n+ ret = nbd_read(s->ioc, *payload, len, errp);\n+ if (ret < 0) {\n+ g_free(*payload);\n+ *payload = NULL;\n+ return ret;\n+ }\n+\n+ return 0;\n+}\n+\n+/* nbd_co_do_receive_one_chunk\n+ * for simple reply:\n+ * set request_ret to received reply error\n+ * if qiov is not NULL: read payload to @qiov\n+ * for structured reply chunk:\n+ * if error chunk: read payload, set @request_ret, do not set @payload\n+ * else if offset_data chunk: read payload data to @qiov, do not set @payload\n+ * else: read payload to @payload\n+ *\n+ * If function fails, @errp contains corresponding error message, and the\n+ * connection with the server is suspect. If it returns 0, then the\n+ * transaction succeeded (although @request_ret may be a negative errno\n+ * corresponding to the server's error reply), and errp is unchanged.\n+ */\n+static coroutine_fn int nbd_co_do_receive_one_chunk(\n+ NBDClientSession *s, uint64_t handle, bool only_structured,\n+ int *request_ret, QEMUIOVector *qiov, void **payload, Error **errp)\n {\n int ret;\n int i = HANDLE_TO_INDEX(s, handle);\n+ void *local_payload = NULL;\n+ NBDStructuredReplyChunk *chunk;\n+\n+ if (payload) {\n+ *payload = NULL;\n+ }\n+ *request_ret = 0;\n\n /* Wait until we're woken up by nbd_read_reply_entry. */\n s->requests[i].receiving = true;\n qemu_coroutine_yield();\n s->requests[i].receiving = false;\n if (!s->ioc || s->quit) {\n- ret = -EIO;\n+ error_setg(errp, \"Connection closed\");\n+ return -EIO;\n+ }\n+\n+ assert(s->reply.handle == handle);\n+\n+ if (nbd_reply_is_simple(&s->reply)) {\n+ if (only_structured) {\n+ error_setg(errp, \"Protocol error: simple reply when structured \"\n+ \"reply chunk was expected\");\n+ return -EINVAL;\n+ }\n+\n+ *request_ret = -nbd_errno_to_system_errno(s->reply.simple.error);\n+ if (*request_ret < 0 || !qiov) {\n+ return 0;\n+ }\n+\n+ return qio_channel_readv_all(s->ioc, qiov->iov, qiov->niov,\n+ errp) < 0 ? -EIO : 0;\n+ }\n+\n+ /* handle structured reply chunk */\n+ assert(s->info.structured_reply);\n+ chunk = &s->reply.structured;\n+\n+ if (chunk->type == NBD_REPLY_TYPE_NONE) {\n+ if (!(chunk->flags & NBD_REPLY_FLAG_DONE)) {\n+ error_setg(errp, \"Protocol error: NBD_REPLY_TYPE_NONE chunk without\"\n+ \"NBD_REPLY_FLAG_DONE flag set\");\n+ return -EINVAL;\n+ }\n+ return 0;\n+ }\n+\n+ if (chunk->type == NBD_REPLY_TYPE_OFFSET_DATA) {\n+ if (!qiov) {\n+ error_setg(errp, \"Unexpected NBD_REPLY_TYPE_OFFSET_DATA chunk\");\n+ return -EINVAL;\n+ }\n+\n+ return nbd_co_receive_offset_data_payload(s, s->requests[i].offset,\n+ qiov, errp);\n+ }\n+\n+ if (nbd_reply_type_is_error(chunk->type)) {\n+ payload = &local_payload;\n+ }\n+\n+ ret = nbd_co_receive_structured_payload(s, payload, errp);\n+ if (ret < 0) {\n+ return ret;\n+ }\n+\n+ if (nbd_reply_type_is_error(chunk->type)) {\n+ ret = nbd_parse_error_payload(chunk, local_payload, request_ret, errp);\n+ g_free(local_payload);\n+ return ret;\n+ }\n+\n+ return 0;\n+}\n+\n+/* nbd_co_receive_one_chunk\n+ * Read reply, wake up read_reply_co and set s->quit if needed.\n+ * Return value is a fatal error code or normal nbd reply error code\n+ */\n+static coroutine_fn int nbd_co_receive_one_chunk(\n+ NBDClientSession *s, uint64_t handle, bool only_structured,\n+ QEMUIOVector *qiov, NBDReply *reply, void **payload, Error **errp)\n+{\n+ int request_ret;\n+ int ret = nbd_co_do_receive_one_chunk(s, handle, only_structured,\n+ &request_ret, qiov, payload, errp);\n+\n+ if (ret < 0) {\n+ s->quit = true;\n } else {\n- assert(s->reply.handle == handle);\n- ret = -nbd_errno_to_system_errno(s->reply.simple.error);\n- if (qiov && ret == 0) {\n- if (qio_channel_readv_all(s->ioc, qiov->iov, qiov->niov,\n- NULL) < 0) {\n- ret = -EIO;\n- s->quit = true;\n- }\n+ /* For assert at loop start in nbd_read_reply_entry */\n+ if (reply) {\n+ *reply = s->reply;\n }\n-\n- /* Tell the read handler to read another header. */\n s->reply.handle = 0;\n+ ret = request_ret;\n }\n\n- s->requests[i].coroutine = NULL;\n-\n- /* Kick the read_reply_co to get the next reply. */\n if (s->read_reply_co) {\n aio_co_wake(s->read_reply_co);\n }\n\n+ return ret;\n+}\n+\n+typedef struct NBDReplyChunkIter {\n+ int ret;\n+ Error *err;\n+ bool done, only_structured;\n+} NBDReplyChunkIter;\n+\n+static void nbd_iter_error(NBDReplyChunkIter *iter, bool fatal,\n+ int ret, Error **local_err)\n+{\n+ assert(ret < 0);\n+\n+ if (fatal || iter->ret == 0) {\n+ if (iter->ret != 0) {\n+ error_free(iter->err);\n+ iter->err = NULL;\n+ }\n+ iter->ret = ret;\n+ error_propagate(&iter->err, *local_err);\n+ } else {\n+ error_free(*local_err);\n+ }\n+\n+ *local_err = NULL;\n+}\n+\n+/* NBD_FOREACH_REPLY_CHUNK\n+ */\n+#define NBD_FOREACH_REPLY_CHUNK(s, iter, handle, structured, \\\n+ qiov, reply, payload) \\\n+ for (iter = (NBDReplyChunkIter) { .only_structured = structured }; \\\n+ nbd_reply_chunk_iter_receive(s, &iter, handle, qiov, reply, payload);)\n+\n+/* nbd_reply_chunk_iter_receive\n+ */\n+static bool nbd_reply_chunk_iter_receive(NBDClientSession *s,\n+ NBDReplyChunkIter *iter,\n+ uint64_t handle,\n+ QEMUIOVector *qiov, NBDReply *reply,\n+ void **payload)\n+{\n+ int ret;\n+ NBDReply local_reply;\n+ NBDStructuredReplyChunk *chunk;\n+ Error *local_err = NULL;\n+ if (s->quit) {\n+ error_setg(&local_err, \"Connection closed\");\n+ nbd_iter_error(iter, true, -EIO, &local_err);\n+ goto break_loop;\n+ }\n+\n+ if (iter->done) {\n+ /* Previous iteration was last. */\n+ goto break_loop;\n+ }\n+\n+ if (reply == NULL) {\n+ reply = &local_reply;\n+ }\n+\n+ ret = nbd_co_receive_one_chunk(s, handle, iter->only_structured,\n+ qiov, reply, payload, &local_err);\n+ if (ret < 0) {\n+ /* If it is a fatal error s->quit is set by nbd_co_receive_one_chunk */\n+ nbd_iter_error(iter, s->quit, ret, &local_err);\n+ }\n+\n+ /* Do not execute the body of NBD_FOREACH_REPLY_CHUNK for simple reply. */\n+ if (nbd_reply_is_simple(&s->reply) || s->quit) {\n+ goto break_loop;\n+ }\n+\n+ chunk = &reply->structured;\n+ iter->only_structured = true;\n+\n+ if (chunk->type == NBD_REPLY_TYPE_NONE) {\n+ /* NBD_REPLY_FLAG_DONE is already checked in nbd_co_receive_one_chunk */\n+ assert(chunk->flags & NBD_REPLY_FLAG_DONE);\n+ goto break_loop;\n+ }\n+\n+ if (chunk->flags & NBD_REPLY_FLAG_DONE) {\n+ /* This iteration is last. */\n+ iter->done = true;\n+ }\n+\n+ /* Execute the loop body */\n+ return true;\n+\n+break_loop:\n+ s->requests[HANDLE_TO_INDEX(s, handle)].coroutine = NULL;\n+\n qemu_co_mutex_lock(&s->send_mutex);\n s->in_flight--;\n qemu_co_queue_next(&s->free_sema);\n qemu_co_mutex_unlock(&s->send_mutex);\n\n- return ret;\n+ return false;\n }\n\n-static int nbd_co_request(BlockDriverState *bs,\n- NBDRequest *request,\n- QEMUIOVector *qiov)\n+static int nbd_co_receive_return_code(NBDClientSession *s, uint64_t handle,\n+ Error **errp)\n+{\n+ NBDReplyChunkIter iter;\n+\n+ NBD_FOREACH_REPLY_CHUNK(s, iter, handle, false, NULL, NULL, NULL) {\n+ /* nbd_reply_chunk_iter_receive does all the work */\n+ }\n+\n+ error_propagate(errp, iter.err);\n+ return iter.ret;\n+}\n+\n+static int nbd_co_receive_cmdread_reply(NBDClientSession *s, uint64_t handle,\n+ uint64_t offset, QEMUIOVector *qiov,\n+ Error **errp)\n+{\n+ NBDReplyChunkIter iter;\n+ NBDReply reply;\n+ void *payload = NULL;\n+ Error *local_err = NULL;\n+\n+ NBD_FOREACH_REPLY_CHUNK(s, iter, handle, s->info.structured_reply,\n+ qiov, &reply, &payload)\n+ {\n+ int ret;\n+ NBDStructuredReplyChunk *chunk = &reply.structured;\n+\n+ assert(nbd_reply_is_structured(&reply));\n+\n+ switch (chunk->type) {\n+ case NBD_REPLY_TYPE_OFFSET_DATA:\n+ /* special cased in nbd_co_receive_one_chunk, data is already\n+ * in qiov */\n+ break;\n+ case NBD_REPLY_TYPE_OFFSET_HOLE:\n+ ret = nbd_parse_offset_hole_payload(&reply.structured, payload,\n+ offset, qiov, &local_err);\n+ if (ret < 0) {\n+ s->quit = true;\n+ nbd_iter_error(&iter, true, ret, &local_err);\n+ }\n+ break;\n+ default:\n+ if (!nbd_reply_type_is_error(chunk->type)) {\n+ /* not allowed reply type */\n+ s->quit = true;\n+ error_setg(&local_err,\n+ \"Unexpected reply type: %d (%s) for CMD_READ\",\n+ chunk->type, nbd_reply_type_lookup(chunk->type));\n+ nbd_iter_error(&iter, true, -EINVAL, &local_err);\n+ }\n+ }\n+\n+ g_free(payload);\n+ payload = NULL;\n+ }\n+\n+ error_propagate(errp, iter.err);\n+ return iter.ret;\n+}\n+\n+static int nbd_co_request(BlockDriverState *bs, NBDRequest *request,\n+ QEMUIOVector *write_qiov)\n {\n- NBDClientSession *client = nbd_get_client_session(bs);\n int ret;\n+ Error *local_err = NULL;\n+ NBDClientSession *client = nbd_get_client_session(bs);\n\n- if (qiov) {\n- assert(request->type == NBD_CMD_WRITE || request->type == NBD_CMD_READ);\n- assert(request->len == iov_size(qiov->iov, qiov->niov));\n+ assert(request->type != NBD_CMD_READ);\n+ if (write_qiov) {\n+ assert(request->type == NBD_CMD_WRITE);\n+ assert(request->len == iov_size(write_qiov->iov, write_qiov->niov));\n } else {\n- assert(request->type != NBD_CMD_WRITE && request->type != NBD_CMD_READ);\n+ assert(request->type != NBD_CMD_WRITE);\n }\n- ret = nbd_co_send_request(bs, request,\n- request->type == NBD_CMD_WRITE ? qiov : NULL);\n+ ret = nbd_co_send_request(bs, request, write_qiov);\n if (ret < 0) {\n return ret;\n }\n\n- return nbd_co_receive_reply(client, request->handle,\n- request->type == NBD_CMD_READ ? qiov : NULL);\n+ ret = nbd_co_receive_return_code(client, request->handle, &local_err);\n+ if (local_err) {\n+ error_report_err(local_err);\n+ }\n+ return ret;\n }\n\n int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,\n uint64_t bytes, QEMUIOVector *qiov, int flags)\n {\n+ int ret;\n+ Error *local_err = NULL;\n+ NBDClientSession *client = nbd_get_client_session(bs);\n NBDRequest request = {\n .type = NBD_CMD_READ,\n .from = offset,\n@@ -259,7 +674,17 @@ int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,\n assert(bytes <= NBD_MAX_BUFFER_SIZE);\n assert(!flags);\n\n- return nbd_co_request(bs, &request, qiov);\n+ ret = nbd_co_send_request(bs, &request, NULL);\n+ if (ret < 0) {\n+ return ret;\n+ }\n+\n+ ret = nbd_co_receive_cmdread_reply(client, request.handle, offset, qiov,\n+ &local_err);\n+ if (ret < 0) {\n+ error_report_err(local_err);\n+ }\n+ return ret;\n }\n\n int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,\n@@ -381,6 +806,7 @@ int nbd_client_init(BlockDriverState *bs,\n qio_channel_set_blocking(QIO_CHANNEL(sioc), true, NULL);\n\n client->info.request_sizes = true;\n+ client->info.structured_reply = true;\n ret = nbd_receive_negotiate(QIO_CHANNEL(sioc), export,\n tlscreds, hostname,\n &client->ioc, &client->info, errp);\ndiff --git a/nbd/client.c b/nbd/client.c\nindex 4f0745f601..3d680e63e1 100644\n--- a/nbd/client.c\n+++ b/nbd/client.c\n@@ -602,9 +602,11 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name,\n uint64_t magic;\n int rc;\n bool zeroes = true;\n+ bool structured_reply = info->structured_reply;\n\n trace_nbd_receive_negotiate(tlscreds, hostname ? hostname : \"\");\n\n+ info->structured_reply = false;\n rc = -EINVAL;\n\n if (outioc) {\n@@ -685,6 +687,16 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name,\n if (fixedNewStyle) {\n int result;\n\n+ if (structured_reply) {\n+ result = nbd_request_simple_option(ioc,\n+ NBD_OPT_STRUCTURED_REPLY,\n+ errp);\n+ if (result < 0) {\n+ goto fail;\n+ }\n+ info->structured_reply = result == 1;\n+ }\n+\n /* Try NBD_OPT_GO first - if it works, we are done (it\n * also gives us a good message if the server requires\n * TLS). If it is not available, fall back to\ndiff --git a/tests/qemu-iotests/083.out b/tests/qemu-iotests/083.out\nindex 25dde519e3..be6079d27e 100644\n--- a/tests/qemu-iotests/083.out\n+++ b/tests/qemu-iotests/083.out\n@@ -41,6 +41,7 @@ can't open device nbd+tcp://127.0.0.1:PORT/foo\n\n === Check disconnect after neg2 ===\n\n+Connection closed\n read failed: Input/output error\n\n === Check disconnect 8 neg2 ===\n@@ -53,32 +54,39 @@ can't open device nbd+tcp://127.0.0.1:PORT/foo\n\n === Check disconnect before request ===\n\n+Connection closed\n read failed: Input/output error\n\n === Check disconnect after request ===\n\n+Connection closed\n read failed: Input/output error\n\n === Check disconnect before reply ===\n\n+Connection closed\n read failed: Input/output error\n\n === Check disconnect after reply ===\n\n+Unexpected end-of-file before all bytes were read\n read failed: Input/output error\n\n === Check disconnect 4 reply ===\n\n Unexpected end-of-file before all bytes were read\n+Connection closed\n read failed: Input/output error\n\n === Check disconnect 8 reply ===\n\n Unexpected end-of-file before all bytes were read\n+Connection closed\n read failed: Input/output error\n\n === Check disconnect before data ===\n\n+Unexpected end-of-file before all bytes were read\n read failed: Input/output error\n\n === Check disconnect after data ===\n@@ -108,6 +116,7 @@ can't open device nbd+tcp://127.0.0.1:PORT/\n\n === Check disconnect after neg-classic ===\n\n+Connection closed\n read failed: Input/output error\n\n === Check disconnect before neg1 ===\n@@ -168,28 +177,34 @@ read failed: Input/output error\n\n === Check disconnect after request ===\n\n+Connection closed\n read failed: Input/output error\n\n === Check disconnect before reply ===\n\n+Connection closed\n read failed: Input/output error\n\n === Check disconnect after reply ===\n\n+Unexpected end-of-file before all bytes were read\n read failed: Input/output error\n\n === Check disconnect 4 reply ===\n\n Unexpected end-of-file before all bytes were read\n+Connection closed\n read failed: Input/output error\n\n === Check disconnect 8 reply ===\n\n Unexpected end-of-file before all bytes were read\n+Connection closed\n read failed: Input/output error\n\n === Check disconnect before data ===\n\n+Unexpected end-of-file before all bytes were read\n read failed: Input/output error\n\n === Check disconnect after data ===\n","prefixes":["v6","12/12"]}