Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge tag 'pull-nbd-2023-09-25' of https://repo.or.cz/qemu/ericb into… 
… staging

NBD patches through 2023-09-25

- Denis V. Lunev: iotest improvements
- Eric Blake: further work towards 64-bit NBD extensions

# -----BEGIN PGP SIGNATURE-----
#
# iQEzBAABCAAdFiEEccLMIrHEYCkn0vOqp6FrSiUnQ2oFAmUR2MUACgkQp6FrSiUn
# Q2q6jAf+PT65XzMAhgKvu1vIeMSQqyCocNB2MCOzNp+46uB9bNbPPLQSH2EX+t6p
# kQfHyHUl4YMi0EqgCfodiewlaUKeMxP3cPWMGYaYZ16uNMOIYL1boreDAcM25rb5
# P3TV3DAWTWSclUxrkTC2DxAIBPgsPsGG/2daqOMDEdinxlIywCMJDEIHc9gwwd/t
# 7laz9V1cOW9NbQXrM7eTofJKPKIeqZ+w0kvqrf9HBvZl9CqwHADi7xoz9xP+fN+f
# 713ED/hwt0FIlixtIm2/8vu7nn09cu6m9NaKsMOomsYg9Z6wU3ctivViG5NLq3MD
# OOUu51dV8gRRAXAFU5vKb0d93D27zQ==
# =Ik02
# -----END PGP SIGNATURE-----
# gpg: Signature made Mon 25 Sep 2023 15:00:21 EDT
# gpg:                using RSA key 71C2CC22B1C4602927D2F3AAA7A16B4A2527436A
# gpg: Good signature from "Eric Blake <eblake@redhat.com>" [full]
# gpg:                 aka "Eric Blake (Free Software Programmer) <ebb9@byu.net>" [full]
# gpg:                 aka "[jpeg image of size 6874]" [full]
# Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2  F3AA A7A1 6B4A 2527 436A

* tag 'pull-nbd-2023-09-25' of https://repo.or.cz/qemu/ericb:
  nbd/server: Refactor handling of command sanity checks
  nbd: Prepare for 64-bit request effect lengths
  nbd: Add types for extended headers
  nbd/client: Pass mode through to nbd_send_request
  nbd: Replace bool structured_reply with mode enum
  iotests: improve 'not run' message for nbd-multiconn test
  iotests: use TEST_IMG_FILE instead of TEST_IMG in _require_large_file

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
  • Loading branch information
Stefan Hajnoczi committed Sep 26, 2023
2 parents 494a6a2 + 8db7e2d commit 11a629d
Show file tree
Hide file tree
Showing 12 changed files with 280 additions and 150 deletions.
44 changes: 30 additions & 14 deletions block/nbd.c
View file
Expand Up @@ -339,7 +339,7 @@ int coroutine_fn nbd_co_do_establish_connection(BlockDriverState *bs,
* We have connected, but must fail for other reasons.
* Send NBD_CMD_DISC as a courtesy to the server.
*/
NBDRequest request = { .type = NBD_CMD_DISC };
NBDRequest request = { .type = NBD_CMD_DISC, .mode = s->info.mode };

nbd_send_request(s->ioc, &request);

Expand Down Expand Up @@ -463,7 +463,8 @@ static coroutine_fn int nbd_receive_replies(BDRVNBDState *s, uint64_t cookie)
nbd_channel_error(s, ret);
return ret;
}
if (nbd_reply_is_structured(&s->reply) && !s->info.structured_reply) {
if (nbd_reply_is_structured(&s->reply) &&
s->info.mode < NBD_MODE_STRUCTURED) {
nbd_channel_error(s, -EINVAL);
return -EINVAL;
}
Expand Down Expand Up @@ -519,6 +520,7 @@ nbd_co_send_request(BlockDriverState *bs, NBDRequest *request,

qemu_co_mutex_lock(&s->send_mutex);
request->cookie = INDEX_TO_COOKIE(i);
request->mode = s->info.mode;

assert(s->ioc);

Expand Down Expand Up @@ -608,7 +610,7 @@ static int nbd_parse_offset_hole_payload(BDRVNBDState *s,
static int nbd_parse_blockstatus_payload(BDRVNBDState *s,
NBDStructuredReplyChunk *chunk,
uint8_t *payload, uint64_t orig_length,
NBDExtent *extent, Error **errp)
NBDExtent32 *extent, Error **errp)
{
uint32_t context_id;

Expand Down Expand Up @@ -866,7 +868,7 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
}

/* handle structured reply chunk */
assert(s->info.structured_reply);
assert(s->info.mode >= NBD_MODE_STRUCTURED);
chunk = &s->reply.structured;

if (chunk->type == NBD_REPLY_TYPE_NONE) {
Expand Down Expand Up @@ -1070,7 +1072,8 @@ nbd_co_receive_cmdread_reply(BDRVNBDState *s, uint64_t cookie,
void *payload = NULL;
Error *local_err = NULL;

NBD_FOREACH_REPLY_CHUNK(s, iter, cookie, s->info.structured_reply,
NBD_FOREACH_REPLY_CHUNK(s, iter, cookie,
s->info.mode >= NBD_MODE_STRUCTURED,
qiov, &reply, &payload)
{
int ret;
Expand Down Expand Up @@ -1115,7 +1118,7 @@ nbd_co_receive_cmdread_reply(BDRVNBDState *s, uint64_t cookie,

static int coroutine_fn
nbd_co_receive_blockstatus_reply(BDRVNBDState *s, uint64_t cookie,
uint64_t length, NBDExtent *extent,
uint64_t length, NBDExtent32 *extent,
int *request_ret, Error **errp)
{
NBDReplyChunkIter iter;
Expand Down Expand Up @@ -1302,10 +1305,11 @@ nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int64_t bytes,
NBDRequest request = {
.type = NBD_CMD_WRITE_ZEROES,
.from = offset,
.len = bytes, /* .len is uint32_t actually */
.len = bytes,
};

assert(bytes <= UINT32_MAX); /* rely on max_pwrite_zeroes */
/* rely on max_pwrite_zeroes */
assert(bytes <= UINT32_MAX || s->info.mode >= NBD_MODE_EXTENDED);

assert(!(s->info.flags & NBD_FLAG_READ_ONLY));
if (!(s->info.flags & NBD_FLAG_SEND_WRITE_ZEROES)) {
Expand Down Expand Up @@ -1352,10 +1356,11 @@ nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes)
NBDRequest request = {
.type = NBD_CMD_TRIM,
.from = offset,
.len = bytes, /* len is uint32_t */
.len = bytes,
};

assert(bytes <= UINT32_MAX); /* rely on max_pdiscard */
/* rely on max_pdiscard */
assert(bytes <= UINT32_MAX || s->info.mode >= NBD_MODE_EXTENDED);

assert(!(s->info.flags & NBD_FLAG_READ_ONLY));
if (!(s->info.flags & NBD_FLAG_SEND_TRIM) || !bytes) {
Expand All @@ -1370,15 +1375,14 @@ static int coroutine_fn GRAPH_RDLOCK nbd_client_co_block_status(
int64_t *pnum, int64_t *map, BlockDriverState **file)
{
int ret, request_ret;
NBDExtent extent = { 0 };
NBDExtent32 extent = { 0 };
BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
Error *local_err = NULL;

NBDRequest request = {
.type = NBD_CMD_BLOCK_STATUS,
.from = offset,
.len = MIN(QEMU_ALIGN_DOWN(INT_MAX, bs->bl.request_alignment),
MIN(bytes, s->info.size - offset)),
.len = MIN(bytes, s->info.size - offset),
.flags = NBD_CMD_FLAG_REQ_ONE,
};

Expand All @@ -1388,6 +1392,10 @@ static int coroutine_fn GRAPH_RDLOCK nbd_client_co_block_status(
*file = bs;
return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID;
}
if (s->info.mode < NBD_MODE_EXTENDED) {
request.len = MIN(QEMU_ALIGN_DOWN(INT_MAX, bs->bl.request_alignment),
request.len);
}

/*
* Work around the fact that the block layer doesn't do
Expand Down Expand Up @@ -1463,7 +1471,7 @@ static void nbd_yank(void *opaque)
static void nbd_client_close(BlockDriverState *bs)
{
BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
NBDRequest request = { .type = NBD_CMD_DISC };
NBDRequest request = { .type = NBD_CMD_DISC, .mode = s->info.mode };

if (s->ioc) {
nbd_send_request(s->ioc, &request);
Expand Down Expand Up @@ -1952,6 +1960,14 @@ static void nbd_refresh_limits(BlockDriverState *bs, Error **errp)
bs->bl.max_pwrite_zeroes = max;
bs->bl.max_transfer = max;

/*
* Assume that if the server supports extended headers, it also
* supports unlimited size zero and trim commands.
*/
if (s->info.mode >= NBD_MODE_EXTENDED) {
bs->bl.max_pdiscard = bs->bl.max_pwrite_zeroes = 0;
}

if (s->info.opt_block &&
s->info.opt_block > bs->bl.opt_transfer) {
bs->bl.opt_transfer = s->info.opt_block;
Expand Down
2 changes: 1 addition & 1 deletion block/trace-events
View file
Expand Up @@ -167,7 +167,7 @@ iscsi_xcopy(void *src_lun, uint64_t src_off, void *dst_lun, uint64_t dst_off, ui
nbd_parse_blockstatus_compliance(const char *err) "ignoring extra data from non-compliant server: %s"
nbd_structured_read_compliance(const char *type) "server sent non-compliant unaligned read %s chunk"
nbd_read_reply_entry_fail(int ret, const char *err) "ret = %d, err: %s"
nbd_co_request_fail(uint64_t from, uint32_t len, uint64_t handle, uint16_t flags, uint16_t type, const char *name, int ret, const char *err) "Request failed { .from = %" PRIu64", .len = %" PRIu32 ", .handle = %" PRIu64 ", .flags = 0x%" PRIx16 ", .type = %" PRIu16 " (%s) } ret = %d, err: %s"
nbd_co_request_fail(uint64_t from, uint64_t len, uint64_t handle, uint16_t flags, uint16_t type, const char *name, int ret, const char *err) "Request failed { .from = %" PRIu64", .len = %" PRIu64 ", .handle = %" PRIu64 ", .flags = 0x%" PRIx16 ", .type = %" PRIu16 " (%s) } ret = %d, err: %s"
nbd_client_handshake(const char *export_name) "export '%s'"
nbd_client_handshake_success(const char *export_name) "export '%s'"
nbd_reconnect_attempt(unsigned in_flight) "in_flight %u"
Expand Down
142 changes: 98 additions & 44 deletions include/block/nbd.h
View file
Expand Up @@ -60,20 +60,22 @@ typedef enum NBDMode {
NBD_MODE_EXPORT_NAME, /* newstyle but only OPT_EXPORT_NAME safe */
NBD_MODE_SIMPLE, /* newstyle but only simple replies */
NBD_MODE_STRUCTURED, /* newstyle, structured replies enabled */
/* TODO add NBD_MODE_EXTENDED */
NBD_MODE_EXTENDED, /* newstyle, extended headers enabled */
} NBDMode;

/* Transmission phase structs
*
* Note: these are _NOT_ the same as the network representation of an NBD
* request and reply!
/* Transmission phase structs */

/*
* Note: NBDRequest is _NOT_ the same as the network representation of an NBD
* request!
*/
typedef struct NBDRequest {
uint64_t cookie;
uint64_t from;
uint32_t len;
uint64_t from; /* Offset touched by the command */
uint64_t len; /* Effect length; 32 bit limit without extended headers */
uint16_t flags; /* NBD_CMD_FLAG_* */
uint16_t type; /* NBD_CMD_* */
uint16_t type; /* NBD_CMD_* */
NBDMode mode; /* Determines which network representation to use */
} NBDRequest;

typedef struct NBDSimpleReply {
Expand All @@ -91,20 +93,36 @@ 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 cookie; /* 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 @cookie 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 @cookie 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;
uint64_t cookie;
} QEMU_PACKED;
};
} NBDReply;
QEMU_BUILD_BUG_ON(offsetof(NBDReply, simple.cookie) !=
offsetof(NBDReply, cookie));
QEMU_BUILD_BUG_ON(offsetof(NBDReply, structured.cookie) !=
offsetof(NBDReply, cookie));
QEMU_BUILD_BUG_ON(offsetof(NBDReply, extended.cookie) !=
offsetof(NBDReply, cookie));

/* Header of chunk for NBD_REPLY_TYPE_OFFSET_DATA */
typedef struct NBDStructuredReadData {
Expand All @@ -131,14 +149,34 @@ typedef struct NBDStructuredError {
typedef struct NBDStructuredMeta {
/* header's length >= 12 (at least one extent) */
uint32_t context_id;
/* extents follows */
/* NBDExtent32 extents[] follows, array length implied by header */
} QEMU_PACKED NBDStructuredMeta;

/* Extent chunk for NBD_REPLY_TYPE_BLOCK_STATUS */
typedef struct NBDExtent {
/* Extent array element for NBD_REPLY_TYPE_BLOCK_STATUS */
typedef struct NBDExtent32 {
uint32_t length;
uint32_t flags; /* NBD_STATE_* */
} QEMU_PACKED NBDExtent;
} QEMU_PACKED NBDExtent32;

/* Header of NBD_REPLY_TYPE_BLOCK_STATUS_EXT */
typedef struct NBDExtendedMeta {
/* header's length >= 24 (at least one extent) */
uint32_t context_id;
uint32_t count; /* header length must be count * 16 + 8 */
/* NBDExtent64 extents[count] follows */
} QEMU_PACKED NBDExtendedMeta;

/* Extent array element for NBD_REPLY_TYPE_BLOCK_STATUS_EXT */
typedef struct NBDExtent64 {
uint64_t length;
uint64_t flags; /* NBD_STATE_* */
} QEMU_PACKED NBDExtent64;

/* Client payload for limiting NBD_CMD_BLOCK_STATUS reply */
typedef struct NBDBlockStatusPayload {
uint64_t effect_length;
/* uint32_t ids[] follows, array length implied by header */
} QEMU_PACKED NBDBlockStatusPayload;

/* Transmission (export) flags: sent from server to client during handshake,
but describe what will happen during transmission */
Expand All @@ -156,20 +194,22 @@ enum {
NBD_FLAG_SEND_RESIZE_BIT = 9, /* Send resize */
NBD_FLAG_SEND_CACHE_BIT = 10, /* Send CACHE (prefetch) */
NBD_FLAG_SEND_FAST_ZERO_BIT = 11, /* FAST_ZERO flag for WRITE_ZEROES */
NBD_FLAG_BLOCK_STAT_PAYLOAD_BIT = 12, /* PAYLOAD flag for BLOCK_STATUS */
};

#define NBD_FLAG_HAS_FLAGS (1 << NBD_FLAG_HAS_FLAGS_BIT)
#define NBD_FLAG_READ_ONLY (1 << NBD_FLAG_READ_ONLY_BIT)
#define NBD_FLAG_SEND_FLUSH (1 << NBD_FLAG_SEND_FLUSH_BIT)
#define NBD_FLAG_SEND_FUA (1 << NBD_FLAG_SEND_FUA_BIT)
#define NBD_FLAG_ROTATIONAL (1 << NBD_FLAG_ROTATIONAL_BIT)
#define NBD_FLAG_SEND_TRIM (1 << NBD_FLAG_SEND_TRIM_BIT)
#define NBD_FLAG_SEND_WRITE_ZEROES (1 << NBD_FLAG_SEND_WRITE_ZEROES_BIT)
#define NBD_FLAG_SEND_DF (1 << NBD_FLAG_SEND_DF_BIT)
#define NBD_FLAG_CAN_MULTI_CONN (1 << NBD_FLAG_CAN_MULTI_CONN_BIT)
#define NBD_FLAG_SEND_RESIZE (1 << NBD_FLAG_SEND_RESIZE_BIT)
#define NBD_FLAG_SEND_CACHE (1 << NBD_FLAG_SEND_CACHE_BIT)
#define NBD_FLAG_SEND_FAST_ZERO (1 << NBD_FLAG_SEND_FAST_ZERO_BIT)
#define NBD_FLAG_HAS_FLAGS (1 << NBD_FLAG_HAS_FLAGS_BIT)
#define NBD_FLAG_READ_ONLY (1 << NBD_FLAG_READ_ONLY_BIT)
#define NBD_FLAG_SEND_FLUSH (1 << NBD_FLAG_SEND_FLUSH_BIT)
#define NBD_FLAG_SEND_FUA (1 << NBD_FLAG_SEND_FUA_BIT)
#define NBD_FLAG_ROTATIONAL (1 << NBD_FLAG_ROTATIONAL_BIT)
#define NBD_FLAG_SEND_TRIM (1 << NBD_FLAG_SEND_TRIM_BIT)
#define NBD_FLAG_SEND_WRITE_ZEROES (1 << NBD_FLAG_SEND_WRITE_ZEROES_BIT)
#define NBD_FLAG_SEND_DF (1 << NBD_FLAG_SEND_DF_BIT)
#define NBD_FLAG_CAN_MULTI_CONN (1 << NBD_FLAG_CAN_MULTI_CONN_BIT)
#define NBD_FLAG_SEND_RESIZE (1 << NBD_FLAG_SEND_RESIZE_BIT)
#define NBD_FLAG_SEND_CACHE (1 << NBD_FLAG_SEND_CACHE_BIT)
#define NBD_FLAG_SEND_FAST_ZERO (1 << NBD_FLAG_SEND_FAST_ZERO_BIT)
#define NBD_FLAG_BLOCK_STAT_PAYLOAD (1 << NBD_FLAG_BLOCK_STAT_PAYLOAD_BIT)

/* New-style handshake (global) flags, sent from server to client, and
control what will happen during handshake phase. */
Expand All @@ -192,6 +232,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))
Expand All @@ -209,6 +250,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
Expand All @@ -217,12 +260,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 {
Expand All @@ -248,22 +293,31 @@ enum {
*/
#define NBD_MAX_STRING_SIZE 4096

/* Two types of reply structures */
/* Two types of request structures, a given client will only use 1 */
#define NBD_REQUEST_MAGIC 0x25609513
#define NBD_EXTENDED_REQUEST_MAGIC 0x21e41c71

/*
* 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 */
/* Chunk reply flags (for structured and extended replies) */
#define NBD_REPLY_FLAG_DONE (1 << 0) /* This reply-chunk is last */

/* Structured reply types */
/* Chunk 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)
Expand Down Expand Up @@ -305,7 +359,7 @@ typedef struct NBDExportInfo {

/* In-out fields, set by client before nbd_receive_negotiate() and
* updated by server results during nbd_receive_negotiate() */
bool structured_reply;
NBDMode mode; /* input maximum mode tolerated; output actual mode chosen */
bool base_allocation; /* base:allocation context for NBD_CMD_BLOCK_STATUS */

/* Set by server results during nbd_receive_negotiate() and
Expand Down

0 comments on commit 11a629d

Please sign in to comment.