[Libguestfs] [PATCH v3 09/14] nbd/server: Initial support for extended headers
Vladimir Sementsov-Ogievskiy
vsementsov at yandex-team.ru
Wed May 31 14:46:55 UTC 2023
On 15.05.23 22:53, Eric Blake wrote:
> Time to support clients that request extended headers. Now we can
> finally reach the code added across several previous patches.
>
> Even though the NBD spec has been altered to allow us to accept
> NBD_CMD_READ larger than the max payload size (provided our response
> is a hole or broken up over more than one data chunk), we are not
> planning to take advantage of that, and continue to cap NBD_CMD_READ
> to 32M regardless of header size.
>
> For NBD_CMD_WRITE_ZEROES and NBD_CMD_TRIM, the block layer already
> supports 64-bit operations without any effort on our part. For
> NBD_CMD_BLOCK_STATUS, the client's length is a hint, and the previous
> patch took care of implementing the required
> NBD_REPLY_TYPE_BLOCK_STATUS_EXT.
>
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
> nbd/nbd-internal.h | 5 +-
[..]
>
> static inline void set_be_simple_reply(NBDClient *client, struct iovec *iov,
> - uint64_t error, NBDRequest *request)
> + uint32_t error, NBDStructuredError *err,
> + NBDRequest *request)
> {
> - NBDSimpleReply *reply = iov->iov_base;
> + if (client->header_style >= NBD_HEADER_EXTENDED) {
> + NBDExtendedReplyChunk *chunk = iov->iov_base;
>
> - iov->iov_len = sizeof(*reply);
> - stl_be_p(&reply->magic, NBD_SIMPLE_REPLY_MAGIC);
> - stl_be_p(&reply->error, error);
> - stq_be_p(&reply->handle, request->handle);
> + iov->iov_len = sizeof(*chunk);
> + stl_be_p(&chunk->magic, NBD_EXTENDED_REPLY_MAGIC);
> + stw_be_p(&chunk->flags, NBD_REPLY_FLAG_DONE);
> + stq_be_p(&chunk->handle, request->handle);
> + stq_be_p(&chunk->offset, request->from);
> + if (error) {
> + assert(!iov[1].iov_base);
> + iov[1].iov_base = err;
> + iov[1].iov_len = sizeof(*err);
> + stw_be_p(&chunk->type, NBD_REPLY_TYPE_ERROR);
> + stq_be_p(&chunk->length, sizeof(*err));
> + stl_be_p(&err->error, error);
> + stw_be_p(&err->message_length, 0);
> + } else {
> + stw_be_p(&chunk->type, NBD_REPLY_TYPE_NONE);
> + stq_be_p(&chunk->length, 0);
> + }
> + } else {
> + NBDSimpleReply *reply = iov->iov_base;
> +
> + iov->iov_len = sizeof(*reply);
> + stl_be_p(&reply->magic, NBD_SIMPLE_REPLY_MAGIC);
> + stl_be_p(&reply->error, error);
> + stq_be_p(&reply->handle, request->handle);
> + }
> }
>
> static int coroutine_fn nbd_co_send_simple_reply(NBDClient *client,
> @@ -1906,30 +1966,44 @@ static int coroutine_fn nbd_co_send_simple_reply(NBDClient *client,
So, that's not _simple_ now.. The function should be renamed. As well as set_be_simple_reply(). _simple_or_extended_ ? a bit too long. But continuing to use "simple" is in bad relation with use of "simple" word in specification.
Probably better to update callers? The only caller isi nbd_send_generic_reply(). So, could we just add nbd_co_send_single_extended_reply() to call from nbd_send_generic_reply() in case of EXTENDED?
Also, transformation of set_be_simple_reply() do look like it should be two separate functions.
> {
> NBDReply hdr;
> int nbd_err = system_errno_to_nbd_errno(error);
> + NBDStructuredError err;
> struct iovec iov[] = {
> {.iov_base = &hdr},
> {.iov_base = data, .iov_len = len}
> };
>
> + assert(!len || !nbd_err);
> trace_nbd_co_send_simple_reply(request->handle, nbd_err,
> nbd_err_lookup(nbd_err), len);
> - set_be_simple_reply(client, &iov[0], nbd_err, request);
> + set_be_simple_reply(client, &iov[0], nbd_err, &err, request);
>
> - return nbd_co_send_iov(client, iov, len ? 2 : 1, errp);
> + return nbd_co_send_iov(client, iov, iov[1].iov_len ? 2 : 1, errp);
> }
>
> static inline void set_be_chunk(NBDClient *client, struct iovec *iov,
> uint16_t flags, uint16_t type,
> NBDRequest *request, uint32_t length)
> {
> - NBDStructuredReplyChunk *chunk = iov->iov_base;
> + if (client->header_style >= NBD_HEADER_EXTENDED) {
> + NBDExtendedReplyChunk *chunk = iov->iov_base;
>
> - iov->iov_len = sizeof(*chunk);
> - stl_be_p(&chunk->magic, NBD_STRUCTURED_REPLY_MAGIC);
> - stw_be_p(&chunk->flags, flags);
> - stw_be_p(&chunk->type, type);
> - stq_be_p(&chunk->handle, request->handle);
> - stl_be_p(&chunk->length, length);
> + iov->iov_len = sizeof(*chunk);
> + stl_be_p(&chunk->magic, NBD_EXTENDED_REPLY_MAGIC);
> + stw_be_p(&chunk->flags, flags);
> + stw_be_p(&chunk->type, type);
> + stq_be_p(&chunk->handle, request->handle);
> + stq_be_p(&chunk->offset, request->from);
> + stq_be_p(&chunk->length, length);
> + } else {
> + NBDStructuredReplyChunk *chunk = iov->iov_base;
> +
> + iov->iov_len = sizeof(*chunk);
> + stl_be_p(&chunk->magic, NBD_STRUCTURED_REPLY_MAGIC);
> + stw_be_p(&chunk->flags, flags);
> + stw_be_p(&chunk->type, type);
> + stq_be_p(&chunk->handle, request->handle);
> + stl_be_p(&chunk->length, length);
> + }
> }
>
> static int coroutine_fn nbd_co_send_structured_done(NBDClient *client,
--
Best regards,
Vladimir
More information about the Libguestfs
mailing list