search for: nbd_reply_type_block_status

On Fri, Mar 03, 2023 at 04:17:40PM -0600, Eric Blake wrote: > On Fri, Dec 16, 2022 at 10:32:01PM +0300, Vladimir Sementsov-Ogievskiy wrote: > > s-o-b line missed. > > I'm not sure if the NBD project has a strict policy on including one, > but I don't mind adding it. I've never required it, mostly because it's something that I myself always forget, too, so,

On Fri, Dec 16, 2022 at 10:32:01PM +0300, Vladimir Sementsov-Ogievskiy wrote: > On 11/15/22 01:46, Eric Blake wrote: > > The spec was silent on how many extents a server could reply with. > > However, both qemu and nbdkit (the two server implementations known to > > have implemented the NBD_CMD_BLOCK_STATUS extension) implement a hard > > cap, and will truncate the

On 11/15/22 01:46, Eric Blake wrote: > The spec was silent on how many extents a server could reply with. > However, both qemu and nbdkit (the two server implementations known to > have implemented the NBD_CMD_BLOCK_STATUS extension) implement a hard > cap, and will truncate the amount of extents in a reply to avoid > sending a client a reply so large that the client would treat it

A compliant server should not send NBD_REPLY_TYPE_BLOCK_STATUS unless we successfully negotiated a meta context. And our default strictness settings refuse to let us send NBD_CMD_BLOCK_STATUS unless we negotiated a meta context. But when you mix non-default settings (using nbd_set_strict to disable STRICT_COMMANDS) to send a block status without having negot...

On Sat, Jul 15, 2023 at 08:49:51PM -0500, Eric Blake wrote: > A compliant server should not send NBD_REPLY_TYPE_BLOCK_STATUS unless > we successfully negotiated a meta context. And our default strictness > settings refuse to let us send NBD_CMD_BLOCK_STATUS unless we > negotiated a meta context. But when you mix non-default settings > (using nbd_set_strict to disable STRICT_COMMANDS) to send a block > st...

...3 insertions(+), 11 deletions(-) diff --git a/generator/states-reply-structured.c b/generator/states-reply-structured.c index 6740400..657106e 100644 --- a/generator/states-reply-structured.c +++ b/generator/states-reply-structured.c @@ -123,7 +123,7 @@ set_error (0, "invalid length in NBD_REPLY_TYPE_BLOCK_STATUS"); return -1; } - if (cmd->extent_fn == NULL) { + if (cmd->cb.fn.extent == NULL) { SET_NEXT_STATE (%.DEAD); set_error (0, "not expecting NBD_REPLY_TYPE_BLOCK_STATUS here"); return -1; @@ -375,7 +375,7 @@ length = be32toh (h->sbuf.s...

.../* per-export flags */ } __attribute__((packed)); +/* NBD_REP_META_CONTEXT reply (follows fixed_new_option_reply). */ +struct fixed_new_option_reply_meta_context { + uint32_t context_id; /* metadata context ID */ + /* followed by a string */ +} __attribute__((packed)); + +/* NBD_REPLY_TYPE_BLOCK_STATUS block descriptor. */ +struct block_descriptor { + uint32_t length; /* length of block */ + uint32_t status_flags; /* block type (hole etc) */ +} __attribute__((packed)); + /* New-style handshake server reply when using NBD_OPT_EXPORT_NAME. * Modern clients use NBD_OPT_GO in...

...ks as follows: - During transmission, a client can then indicate interest in metadata for a given region by way of the `NBD_CMD_BLOCK_STATUS` command, where *offset* and *length* indicate the area of interest. On - success, the server MUST respond with one structured reply chunk of - type `NBD_REPLY_TYPE_BLOCK_STATUS` per metadata context selected - during negotiation, where each reply chunk is a list of one or more - consecutive extents for that context. Each extent comes with a - *flags* field, the semantics of which are defined by the metadata - context. + success, the server MUST respond with one stat...

...assert (cmd->error == 0); diff --git a/generator/states-reply-structured.c b/generator/states-reply-structured.c index a1641d4..85f0775 100644 --- a/generator/states-reply-structured.c +++ b/generator/states-reply-structured.c @@ -157,7 +157,7 @@ set_error (0, "invalid length in NBD_REPLY_TYPE_BLOCK_STATUS"); return 0; } - if (cmd->cb.fn.extent.callback == NULL) { + if (CALLBACK_IS_NULL (cmd->cb.fn.extent)) { SET_NEXT_STATE (%.DEAD); set_error (0, "not expecting NBD_REPLY_TYPE_BLOCK_STATUS here"); return 0; @@ -293,7 +293,8 @@...

...(packed)); /* NBD_REP_META_CONTEXT reply (follows fixed_new_option_reply). */ -struct fixed_new_option_reply_meta_context { +struct nbd_fixed_new_option_reply_meta_context { uint32_t context_id; /* metadata context ID */ /* followed by a string */ } __attribute__((packed)); /* NBD_REPLY_TYPE_BLOCK_STATUS block descriptor. */ -struct block_descriptor { +struct nbd_block_descriptor { uint32_t length; /* length of block */ uint32_t status_flags; /* block type (hole etc) */ } __attribute__((packed)); @@ -144,14 +157,14 @@ struct block_descriptor { /* New-style handshake serv...

...re due to reflowed paragraphs. [----] : patches are identical [####] : number of functional differences between upstream/downstream patch [down] : patch is downstream-only The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively 001/6:[----] [-C] 'spec: Recommend cap on NBD_REPLY_TYPE_BLOCK_STATUS length' 002/6:[down] 'spec: Change maximum block size to maximum payload size' 003/6:[0237] [FC] 'spec: Add NBD_OPT_EXTENDED_HEADERS' 004/6:[----] [-C] 'spec: Allow 64-bit block status results' 005/6:[----] [-C] 'spec: Introduce NBD_FLAG_BLOCK_STATUS_PAYLOAD' 006...

...ize", { @@ -1551,7 +1557,9 @@ pair being the length (in bytes) of the block and the second entry being a status/flags field which is specific to the metadata context. (The number of pairs passed to the function is C<nr_entries/2>.) The NBD protocol document in the section about -C<NBD_REPLY_TYPE_BLOCK_STATUS> describes the meaning of this array. +C<NBD_REPLY_TYPE_BLOCK_STATUS> describes the meaning of this array; +for contexts known to libnbd, B<E<lt>libnbd.hE<gt>> contains constants +beginning with C<LIBNBD_STATE_> that may help decipher the values. It is possible fo...

...ere is no message printed. > (You can use --run '... || echo FAIL' if you don't believe that > qemu-img is exiting with a failure.) > > While preparing this email I noticed that v3.1.0 printed an > error message: > > qemu-img: Protocol error: invalid payload for NBD_REPLY_TYPE_BLOCK_STATUS Which says that qemu used to be noisy at diagnosing the protocol error, and now it is not. Ideally, qemu as a client shouldn't encounter a server that is violating protocol, but when it does, it makes sense for qemu to call attention to the buggy server. That sounds like a regression in qemu...

...onst char *name_of_nbd_reply_flag (int); +#define NBD_REPLY_FLAG_DONE (1<<0) + +/* Structured reply types. */ +extern const char *name_of_nbd_reply_type (int); +#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 3 +#define NBD_REPLY_TYPE_ERROR 32769 +#define NBD_REPLY_TYPE_ERROR_OFFSET 32770 /* NBD commands. */ extern const char *name_of_nbd_cmd (int); diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c index 674f4a4..2f494cd 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -345,7 +345,7...

On 3/23/19 7:58 AM, Richard W.M. Jones wrote: >> A good point here is what happens with the block-status branch[1]. >> Let's see: >> >> $ ./nbdkit memory size=64M --run '/home/rjones/d/qemu/qemu-img convert $nbd /var/tmp/out' >> qemu-img: Payload too large >> nbdkit: memory.1: error: write reply: NBD_CMD_BLOCK_STATUS: Broken pipe >> >>

...T to MAY on the client side. Keeping it at MUST makes sense, because a well-formed server won't be sending unknown reply types. > > Also, should this section say "structured or extended reply"? We use the > same types for both. Makes sense. > > [...] > > +* `NBD_REPLY_TYPE_BLOCK_STATUS_EXT` (6) > > + > > + This chunk type is in the status chunk category. *length* MUST be > > + 8 + (a positive multiple of 16). The semantics of this chunk mirror > > + those of `NBD_REPLY_TYPE_BLOCK_STATUS`, other than the use of a > > + larger *extent length* fie...

.../* per-export flags */ } __attribute__((packed)); +/* NBD_REP_META_CONTEXT reply (follows fixed_new_option_reply). */ +struct fixed_new_option_reply_meta_context { + uint32_t context_id; /* metadata context ID */ + /* followed by a string */ +} __attribute__((packed)); + +/* NBD_REPLY_TYPE_BLOCK_STATUS block descriptor. */ +struct block_descriptor { + uint32_t length; /* length of block */ + uint32_t status_flags; /* block type (hole etc) */ +} __attribute__((packed)); + /* New-style handshake server reply when using NBD_OPT_EXPORT_NAME. * Modern clients use NBD_OPT_GO in...

This just defines the namespace, its contexts and related constants and the only supported one is currently base:allocation. The names of the constants are not very future-proof, but shorter than LIBNBD_META_NS_CONTEXT_BASE_ALLOCATION or similar. Currently the output looks like this: /* "base" namespace */ /* "base" namespace contexts */ /* "base:allocation"

...the day they need to implement the feature. Background reading: - preparatory patches: https://www.redhat.com/archives/libguestfs/2019-March/msg00013.html https://www.redhat.com/archives/libguestfs/2019-March/msg00016.html - NBD protocol, see in particular NBD_CMD_BLOCK_STATUS and NBD_REPLY_TYPE_BLOCK_STATUS: https://github.com/NetworkBlockDevice/nbd/blob/master/doc/proto.md I think we shouldn't modify the pread() callback. If we decide to implement Structured Replies properly at some point in the future we might need to do that, but it's not necessary now. We could introduce a new call...

...with "minimal implementation"). Either way works. > +/* Structured reply types. */ > +extern const char *name_of_nbd_reply_type (int); > +#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 3 > +#define NBD_REPLY_TYPE_ERROR 32769 > +#define NBD_REPLY_TYPE_ERROR_OFFSET 32770 Worth writing these later ones in hex or via a helper macro that does ((1 << 15) | value)? Or would that mess up the generated protocol-to-lookup magic? > +++ b/plugins/nbd/nbd.c > @@ -34...