| Message ID | 20230515195343.1915857-6-eblake@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qemu patches for 64-bit NBD extensions | expand |
On 15.05.23 22:53, Eric Blake wrote: > Add the constants and structs necessary for later patches to start > implementing the NBD_OPT_EXTENDED_HEADERS extension in both the client > and server, matching recent commit e6f3b94a934] in the upstream nbd > project. This patch does not change any existing behavior, but merely > sets the stage. > > This patch does not change the status quo that neither the client nor > server use a packed-struct representation for the request header. > > Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > --- > docs/interop/nbd.txt | 1 + > include/block/nbd.h | 74 ++++++++++++++++++++++++++++++++------------ > nbd/common.c | 10 +++++- > 3 files changed, 65 insertions(+), 20 deletions(-) > > diff --git a/docs/interop/nbd.txt b/docs/interop/nbd.txt > index f5ca25174a6..abaf4c28a96 100644 > --- a/docs/interop/nbd.txt > +++ b/docs/interop/nbd.txt > @@ -69,3 +69,4 @@ NBD_CMD_BLOCK_STATUS for "qemu:dirty-bitmap:", NBD_CMD_CACHE > NBD_CMD_FLAG_FAST_ZERO > * 5.2: NBD_CMD_BLOCK_STATUS for "qemu:allocation-depth" > * 7.1: NBD_FLAG_CAN_MULTI_CONN for shareable writable exports > +* 8.1: NBD_OPT_EXTENDED_HEADERS > diff --git a/include/block/nbd.h b/include/block/nbd.h > index 50626ab2744..d753fb8006f 100644 > --- a/include/block/nbd.h > +++ b/include/block/nbd.h > @@ -87,13 +87,24 @@ typedef struct NBDStructuredReplyChunk { > uint32_t length; /* length of payload */ > } QEMU_PACKED NBDStructuredReplyChunk; > [..] > -/* Extent chunk for NBD_REPLY_TYPE_BLOCK_STATUS */ > +/* Extent array for NBD_REPLY_TYPE_BLOCK_STATUS */ Why? NBDExtent is one extent, not extent array. > typedef struct NBDExtent { > uint32_t length; > uint32_t flags; /* NBD_STATE_* */ > } QEMU_PACKED NBDExtent; > > +/* Header of NBD_REPLY_TYPE_BLOCK_STATUS_EXT */
On Tue, May 30, 2023 at 04:23:46PM +0300, Vladimir Sementsov-Ogievskiy wrote: > On 15.05.23 22:53, Eric Blake wrote: > > Add the constants and structs necessary for later patches to start > > implementing the NBD_OPT_EXTENDED_HEADERS extension in both the client > > and server, matching recent commit e6f3b94a934] in the upstream nbd > > project. This patch does not change any existing behavior, but merely > > sets the stage. > > > > This patch does not change the status quo that neither the client nor > > server use a packed-struct representation for the request header. > > > > Signed-off-by: Eric Blake <eblake@redhat.com> > > > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > > > --- > > docs/interop/nbd.txt | 1 + > > include/block/nbd.h | 74 ++++++++++++++++++++++++++++++++------------ > > nbd/common.c | 10 +++++- > > 3 files changed, 65 insertions(+), 20 deletions(-) > > > > diff --git a/docs/interop/nbd.txt b/docs/interop/nbd.txt > > index f5ca25174a6..abaf4c28a96 100644 > > --- a/docs/interop/nbd.txt > > +++ b/docs/interop/nbd.txt > > @@ -69,3 +69,4 @@ NBD_CMD_BLOCK_STATUS for "qemu:dirty-bitmap:", NBD_CMD_CACHE > > NBD_CMD_FLAG_FAST_ZERO > > * 5.2: NBD_CMD_BLOCK_STATUS for "qemu:allocation-depth" > > * 7.1: NBD_FLAG_CAN_MULTI_CONN for shareable writable exports > > +* 8.1: NBD_OPT_EXTENDED_HEADERS > > diff --git a/include/block/nbd.h b/include/block/nbd.h > > index 50626ab2744..d753fb8006f 100644 > > --- a/include/block/nbd.h > > +++ b/include/block/nbd.h > > @@ -87,13 +87,24 @@ typedef struct NBDStructuredReplyChunk { > > uint32_t length; /* length of payload */ > > } QEMU_PACKED NBDStructuredReplyChunk; > > > > [..] > > > -/* Extent chunk for NBD_REPLY_TYPE_BLOCK_STATUS */ > > +/* Extent array for NBD_REPLY_TYPE_BLOCK_STATUS */ > > Why? NBDExtent is one extent, not extent array. It's not the entire chunk either, because that also includes the header and the metacontext id that are not part of the extent array. Maybe 'Extent array element', which matches our wire layout of: <- chunk -> <- hdr -><- payload -> ... id <- array -> ext[0] ext[1]... > > > typedef struct NBDExtent { > > uint32_t length; > > uint32_t flags; /* NBD_STATE_* */ > > } QEMU_PACKED NBDExtent; > >
On 30.05.23 21:22, Eric Blake wrote: > On Tue, May 30, 2023 at 04:23:46PM +0300, Vladimir Sementsov-Ogievskiy wrote: >> On 15.05.23 22:53, Eric Blake wrote: >>> Add the constants and structs necessary for later patches to start >>> implementing the NBD_OPT_EXTENDED_HEADERS extension in both the client >>> and server, matching recent commit e6f3b94a934] in the upstream nbd >>> project. This patch does not change any existing behavior, but merely >>> sets the stage. >>> >>> This patch does not change the status quo that neither the client nor >>> server use a packed-struct representation for the request header. >>> >>> Signed-off-by: Eric Blake <eblake@redhat.com> >> >> >> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> >> >>> --- >>> docs/interop/nbd.txt | 1 + >>> include/block/nbd.h | 74 ++++++++++++++++++++++++++++++++------------ >>> nbd/common.c | 10 +++++- >>> 3 files changed, 65 insertions(+), 20 deletions(-) >>> >>> diff --git a/docs/interop/nbd.txt b/docs/interop/nbd.txt >>> index f5ca25174a6..abaf4c28a96 100644 >>> --- a/docs/interop/nbd.txt >>> +++ b/docs/interop/nbd.txt >>> @@ -69,3 +69,4 @@ NBD_CMD_BLOCK_STATUS for "qemu:dirty-bitmap:", NBD_CMD_CACHE >>> NBD_CMD_FLAG_FAST_ZERO >>> * 5.2: NBD_CMD_BLOCK_STATUS for "qemu:allocation-depth" >>> * 7.1: NBD_FLAG_CAN_MULTI_CONN for shareable writable exports >>> +* 8.1: NBD_OPT_EXTENDED_HEADERS >>> diff --git a/include/block/nbd.h b/include/block/nbd.h >>> index 50626ab2744..d753fb8006f 100644 >>> --- a/include/block/nbd.h >>> +++ b/include/block/nbd.h >>> @@ -87,13 +87,24 @@ typedef struct NBDStructuredReplyChunk { >>> uint32_t length; /* length of payload */ >>> } QEMU_PACKED NBDStructuredReplyChunk; >>> >> >> [..] >> >>> -/* Extent chunk for NBD_REPLY_TYPE_BLOCK_STATUS */ >>> +/* Extent array for NBD_REPLY_TYPE_BLOCK_STATUS */ >> >> Why? NBDExtent is one extent, not extent array. > > It's not the entire chunk either, because that also includes the > header and the metacontext id that are not part of the extent array. > Maybe 'Extent array element', which matches our wire layout of: Yes, sounds good > > <- chunk -> > <- hdr -><- payload -> > ... id <- array -> > ext[0] ext[1]... > >> >>> typedef struct NBDExtent { >>> uint32_t length; >>> uint32_t flags; /* NBD_STATE_* */ >>> } QEMU_PACKED NBDExtent; >>> >
diff --git a/docs/interop/nbd.txt b/docs/interop/nbd.txt index f5ca25174a6..abaf4c28a96 100644 --- a/docs/interop/nbd.txt +++ b/docs/interop/nbd.txt @@ -69,3 +69,4 @@ NBD_CMD_BLOCK_STATUS for "qemu:dirty-bitmap:", NBD_CMD_CACHE NBD_CMD_FLAG_FAST_ZERO * 5.2: NBD_CMD_BLOCK_STATUS for "qemu:allocation-depth" * 7.1: NBD_FLAG_CAN_MULTI_CONN for shareable writable exports +* 8.1: NBD_OPT_EXTENDED_HEADERS diff --git a/include/block/nbd.h b/include/block/nbd.h index 50626ab2744..d753fb8006f 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -87,13 +87,24 @@ typedef struct NBDStructuredReplyChunk { uint32_t length; /* length of payload */ } QEMU_PACKED NBDStructuredReplyChunk; +typedef struct NBDExtendedReplyChunk { + uint32_t magic; /* NBD_EXTENDED_REPLY_MAGIC */ + uint16_t flags; /* combination of NBD_REPLY_FLAG_* */ + uint16_t type; /* NBD_REPLY_TYPE_* */ + uint64_t handle; /* request handle */ + uint64_t offset; /* request offset */ + uint64_t length; /* length of payload */ +} QEMU_PACKED NBDExtendedReplyChunk; + typedef union NBDReply { NBDSimpleReply simple; NBDStructuredReplyChunk structured; + NBDExtendedReplyChunk extended; struct { - /* @magic and @handle fields have the same offset and size both in - * simple reply and structured reply chunk, so let them be accessible - * without ".simple." or ".structured." specification + /* + * @magic and @handle fields have the same offset and size in all + * forms of replies, so let them be accessible without ".simple.", + * ".structured.", or ".extended." specifications. */ uint32_t magic; uint32_t _skip; @@ -126,15 +137,29 @@ typedef struct NBDStructuredError { typedef struct NBDStructuredMeta { /* header's length >= 12 (at least one extent) */ uint32_t context_id; - /* extents follows */ + /* NBDExtent extents[] follows, array length implied by header */ } QEMU_PACKED NBDStructuredMeta; -/* Extent chunk for NBD_REPLY_TYPE_BLOCK_STATUS */ +/* Extent array for NBD_REPLY_TYPE_BLOCK_STATUS */ typedef struct NBDExtent { uint32_t length; uint32_t flags; /* NBD_STATE_* */ } QEMU_PACKED NBDExtent; +/* Header of NBD_REPLY_TYPE_BLOCK_STATUS_EXT */ +typedef struct NBDStructuredMetaExt { + /* header's length >= 24 (at least one extent) */ + uint32_t context_id; + uint32_t count; /* header length must be count * 16 + 8 */ + /* NBDExtentExt extents[count] follows */ +} QEMU_PACKED NBDStructuredMetaExt; + +/* Extent array for NBD_REPLY_TYPE_BLOCK_STATUS_EXT */ +typedef struct NBDExtentExt { + uint64_t length; + uint64_t flags; /* NBD_STATE_* */ +} QEMU_PACKED NBDExtentExt; + /* Transmission (export) flags: sent from server to client during handshake, but describe what will happen during transmission */ enum { @@ -187,6 +212,7 @@ enum { #define NBD_OPT_STRUCTURED_REPLY (8) #define NBD_OPT_LIST_META_CONTEXT (9) #define NBD_OPT_SET_META_CONTEXT (10) +#define NBD_OPT_EXTENDED_HEADERS (11) /* Option reply types. */ #define NBD_REP_ERR(value) ((UINT32_C(1) << 31) | (value)) @@ -204,6 +230,8 @@ enum { #define NBD_REP_ERR_UNKNOWN NBD_REP_ERR(6) /* Export unknown */ #define NBD_REP_ERR_SHUTDOWN NBD_REP_ERR(7) /* Server shutting down */ #define NBD_REP_ERR_BLOCK_SIZE_REQD NBD_REP_ERR(8) /* Need INFO_BLOCK_SIZE */ +#define NBD_REP_ERR_TOO_BIG NBD_REP_ERR(9) /* Payload size overflow */ +#define NBD_REP_ERR_EXT_HEADER_REQD NBD_REP_ERR(10) /* Need extended headers */ /* Info types, used during NBD_REP_INFO */ #define NBD_INFO_EXPORT 0 @@ -212,12 +240,14 @@ enum { #define NBD_INFO_BLOCK_SIZE 3 /* Request flags, sent from client to server during transmission phase */ -#define NBD_CMD_FLAG_FUA (1 << 0) /* 'force unit access' during write */ -#define NBD_CMD_FLAG_NO_HOLE (1 << 1) /* don't punch hole on zero run */ -#define NBD_CMD_FLAG_DF (1 << 2) /* don't fragment structured read */ -#define NBD_CMD_FLAG_REQ_ONE (1 << 3) /* only one extent in BLOCK_STATUS - * reply chunk */ -#define NBD_CMD_FLAG_FAST_ZERO (1 << 4) /* fail if WRITE_ZEROES is not fast */ +#define NBD_CMD_FLAG_FUA (1 << 0) /* 'force unit access' during write */ +#define NBD_CMD_FLAG_NO_HOLE (1 << 1) /* don't punch hole on zero run */ +#define NBD_CMD_FLAG_DF (1 << 2) /* don't fragment structured read */ +#define NBD_CMD_FLAG_REQ_ONE (1 << 3) \ + /* only one extent in BLOCK_STATUS reply chunk */ +#define NBD_CMD_FLAG_FAST_ZERO (1 << 4) /* fail if WRITE_ZEROES is not fast */ +#define NBD_CMD_FLAG_PAYLOAD_LEN (1 << 5) \ + /* length describes payload, not effect; only with ext header */ /* Supported request types */ enum { @@ -243,12 +273,17 @@ enum { */ #define NBD_MAX_STRING_SIZE 4096 -/* Transmission request structure */ +/* Two types of request structures, a given client will only use 1 */ #define NBD_REQUEST_MAGIC 0x25609513 +#define NBD_EXTENDED_REQUEST_MAGIC 0x21e41c71 -/* Two types of reply structures */ +/* + * Three types of reply structures, but what a client expects depends + * on NBD_OPT_STRUCTURED_REPLY and NBD_OPT_EXTENDED_HEADERS. + */ #define NBD_SIMPLE_REPLY_MAGIC 0x67446698 #define NBD_STRUCTURED_REPLY_MAGIC 0x668e33ef +#define NBD_EXTENDED_REPLY_MAGIC 0x6e8a278c /* Structured reply flags */ #define NBD_REPLY_FLAG_DONE (1 << 0) /* This reply-chunk is last */ @@ -256,12 +291,13 @@ enum { /* Structured reply types */ #define NBD_REPLY_ERR(value) ((1 << 15) | (value)) -#define NBD_REPLY_TYPE_NONE 0 -#define NBD_REPLY_TYPE_OFFSET_DATA 1 -#define NBD_REPLY_TYPE_OFFSET_HOLE 2 -#define NBD_REPLY_TYPE_BLOCK_STATUS 5 -#define NBD_REPLY_TYPE_ERROR NBD_REPLY_ERR(1) -#define NBD_REPLY_TYPE_ERROR_OFFSET NBD_REPLY_ERR(2) +#define NBD_REPLY_TYPE_NONE 0 +#define NBD_REPLY_TYPE_OFFSET_DATA 1 +#define NBD_REPLY_TYPE_OFFSET_HOLE 2 +#define NBD_REPLY_TYPE_BLOCK_STATUS 5 +#define NBD_REPLY_TYPE_BLOCK_STATUS_EXT 6 +#define NBD_REPLY_TYPE_ERROR NBD_REPLY_ERR(1) +#define NBD_REPLY_TYPE_ERROR_OFFSET NBD_REPLY_ERR(2) /* Extent flags for base:allocation in NBD_REPLY_TYPE_BLOCK_STATUS */ #define NBD_STATE_HOLE (1 << 0) diff --git a/nbd/common.c b/nbd/common.c index ddfe7d11837..137466defd2 100644 --- a/nbd/common.c +++ b/nbd/common.c @@ -79,6 +79,8 @@ const char *nbd_opt_lookup(uint32_t opt) return "list meta context"; case NBD_OPT_SET_META_CONTEXT: return "set meta context"; + case NBD_OPT_EXTENDED_HEADERS: + return "extended headers"; default: return "<unknown>"; } @@ -112,6 +114,10 @@ const char *nbd_rep_lookup(uint32_t rep) return "server shutting down"; case NBD_REP_ERR_BLOCK_SIZE_REQD: return "block size required"; + case NBD_REP_ERR_TOO_BIG: + return "option payload too big"; + case NBD_REP_ERR_EXT_HEADER_REQD: + return "extended headers required"; default: return "<unknown>"; } @@ -170,7 +176,9 @@ const char *nbd_reply_type_lookup(uint16_t type) case NBD_REPLY_TYPE_OFFSET_HOLE: return "hole"; case NBD_REPLY_TYPE_BLOCK_STATUS: - return "block status"; + return "block status (32-bit)"; + case NBD_REPLY_TYPE_BLOCK_STATUS_EXT: + return "block status (64-bit)"; case NBD_REPLY_TYPE_ERROR: return "generic error"; case NBD_REPLY_TYPE_ERROR_OFFSET:
Add the constants and structs necessary for later patches to start implementing the NBD_OPT_EXTENDED_HEADERS extension in both the client and server, matching recent commit e6f3b94a934] in the upstream nbd project. This patch does not change any existing behavior, but merely sets the stage. This patch does not change the status quo that neither the client nor server use a packed-struct representation for the request header. Signed-off-by: Eric Blake <eblake@redhat.com> --- docs/interop/nbd.txt | 1 + include/block/nbd.h | 74 ++++++++++++++++++++++++++++++++------------ nbd/common.c | 10 +++++- 3 files changed, 65 insertions(+), 20 deletions(-)