search for: nbd_cmd_block_status

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 negotiated it, coupled with a non-compliant server that responds with status anyways, we can then hit the assertion failure where h-&gt...

...tyle handshake server reply when using NBD_OPT_EXPORT_NAME. * Modern clients use NBD_OPT_GO instead of this. */ @@ -199,10 +212,12 @@ extern const char *name_of_nbd_cmd (int); #define NBD_CMD_FLUSH 3 #define NBD_CMD_TRIM 4 #define NBD_CMD_WRITE_ZEROES 6 +#define NBD_CMD_BLOCK_STATUS 7 extern const char *name_of_nbd_cmd_flag (int); #define NBD_CMD_FLAG_FUA (1<<0) #define NBD_CMD_FLAG_NO_HOLE (1<<1) +#define NBD_CMD_FLAG_REQ_ONE (1<<3) /* Error codes (previously errno). * See http://git.qemu.org/?p=qemu.git;a=commitdiff;h=ca4414804114fd009...

...#define NBD_REPLY_TYPE_ERROR ((1<<15) + 1) #define NBD_REPLY_TYPE_ERROR_OFFSET ((1<<15) + 2) @@ -199,10 +212,12 @@ extern const char *name_of_nbd_cmd (int); #define NBD_CMD_FLUSH 3 #define NBD_CMD_TRIM 4 #define NBD_CMD_WRITE_ZEROES 6 +#define NBD_CMD_BLOCK_STATUS 7 extern const char *name_of_nbd_cmd_flag (int); #define NBD_CMD_FLAG_FUA (1<<0) #define NBD_CMD_FLAG_NO_HOLE (1<<1) +#define NBD_CMD_FLAG_REQ_ONE (1<<3) /* Error codes (previously errno). * See http://git.qemu.org/?p=qemu.git;a=commitdiff;h=ca4414804114fd009...

...struct nbdkit_extents *extents) > +{ > + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock); > + struct structured_reply reply; > + size_t nr_extents = nbdkit_extents_size (extents); > + uint32_t context_id; > + size_t i; > + int r; > + > + assert (cmd == NBD_CMD_BLOCK_STATUS); > + Worth asserting conn->meta_context_base_allocation? > + > + /* Send each block descriptor. */ > + for (i = 0; i < nr_extents; ++i) { Where does the list terminate after 1 extent if REQ_ONE was set? Also, if REQ_ONE is set but the plugin provided coalesced status beyond...

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 > status without having negotiated it, coupled with a non-compliant > server that responds with status anyways, we can then hit the > asse...

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 as a > denial of service attack. Clients currently have no way during > negotiation to request such a limit of the server, s...

I've posted a couple of patches towards the ultimate goal of implementing NBD_CMD_BLOCK_STATUS / base:allocation in nbdkit. Before I can do the final patch I think we need to discuss how this would be exposed to plugins since at the end of the day they need to implement the feature. Background reading: - preparatory patches: https://www.redhat.com/archives/libguestfs/2019-March/msg00...

...e). Clients MUST NOT set the > > `NBD_CMD_FLAG_FAST_ZERO` request flag unless this transmission flag > > is set. > > +- bit 12, `NBD_FLAG_BLOCK_STATUS_PAYLOAD`: Indicates that the server > > + understands the use of the `NBD_CMD_FLAG_PAYLOAD_LEN` flag to > > + `NBD_CMD_BLOCK_STATUS` to allow the client to request that the > > + server filters its response to a specific subset of negotiated > > + metacontext ids passed in via a client payload, rather than the > > + default of replying to all metacontext ids. Servers MUST NOT > > + advertise this bit...

...f (flags & NBD_REPLY_FLAG_DONE) + FREE_CALLBACK (cmd->cb.fn.extent); } else /* Emit a debug message, but ignore it. */ @@ -548,16 +536,10 @@ flags = be16toh (h->sbuf.sr.structured_reply.flags); if (flags & NBD_REPLY_FLAG_DONE) { - if (cmd->type == NBD_CMD_BLOCK_STATUS && cmd->cb.fn.extent.callback) { - if (cmd->cb.fn.extent.free) - cmd->cb.fn.extent.free (cmd->cb.fn.extent.user_data); - } - if (cmd->type == NBD_CMD_READ && cmd->cb.fn.chunk.callback) { - if (cmd->cb.fn.chunk.free) - cmd->cb.fn....

...y send back a > > single chunk anyway. > > Or, we COULD advertise it because we always honor it (but that's a > larger diffstat, and thus at odds with "minimal implementation"). Either > way works. What's also unclear to me is how NBD_CMD_FLAG_DF interacts with NBD_CMD_BLOCK_STATUS. What does it mean for extents which are by their nature fragmented? > > +/* 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_...

On 3/8/19 7:48 AM, Richard W.M. Jones wrote: > I've posted a couple of patches towards the ultimate goal of > implementing NBD_CMD_BLOCK_STATUS / base:allocation in nbdkit. Before > I can do the final patch I think we need to discuss how this would be > exposed to plugins since at the end of the day they need to implement > the feature. Quick thoughts for now; I'll probably have to spend more time on this over the weekend an...

.../* Emit a debug message, but ignore it. */ @@ -530,17 +518,11 @@ return 0; REPLY.STRUCTURED_REPLY.FINISH: - struct command *cmd = h->reply_cmd; uint16_t flags; flags = be16toh (h->sbuf.sr.structured_reply.flags); - if (flags & NBD_REPLY_FLAG_DONE) { - if (cmd->type == NBD_CMD_BLOCK_STATUS) - FREE_CALLBACK (cmd->cb.fn.extent); - if (cmd->type == NBD_CMD_READ) - FREE_CALLBACK (cmd->cb.fn.chunk); + if (flags & NBD_REPLY_FLAG_DONE) SET_NEXT_STATE (%^FINISH_COMMAND); - } else { h->reply_cmd = NULL; SET_NEXT_STATE (%.READY); diff --git a/ge...

...ec 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 amount of extents in a reply to avoid > > sending a client a reply so large that the client would treat it as a > > denial of service attack. Clients currently have no way during > > negotiation to request such a li...

...ny case wouldn't it > be better to use qemu-nbd? > > You just start a new qemu-nbd process instead of faffing around with > configuration files, kill the qemu-nbd process when you're done, and > qemu-nbd supports qcow2 already. That, and qemu-nbd supports extensions such as NBD_CMD_BLOCK_STATUS and NBD_OPT_STRUCTURED_REPLY that nbd-server has not implemented yet; a qemu NBD client talking to a qemu-nbd server is thus going to be able to take advantage of those extensions for better performance that would not be possible with a qemu NBD client talking to an nbd-server instance (at least, n...

v2 was here: https://lists.debian.org/nbd/2022/11/msg00030.html The bulk of the changes since then are: - forbid NBD_OPT_EXPORT_NAME once extended headers are negotiated (Wouter) - consistently use 'maximum payload', rather than a haphazard mix of 'maximum block payload' (Vladimir) At this point, I want to make sure we are happy with the spec before re-posting patches for

...ommand. For NBD_CMD_READ, replies are still implicitly capped by the maximum block payload limits (generally 32M); if you want to know if a hole larger than 32 bits can represent, you'll use BLOCK_STATUS instead of hoping that a large READ will either return a hole or report overflow. But for NBD_CMD_BLOCK_STATUS, it is very useful to be able to report a status extent with a size larger than 32-bits, in some cases even if the client's request was for smaller than 32-bits (such as when it is known that the entire image is not sparse). Thus, the wording chosen here is careful to permit a server to use ei...

...ed even before we finished wiring up plug support). 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 Oh dear. I believe this is actually a bug in the block status code so let's see if I can narrow this down first ... Rich. [1] https://github.com/rwmjones/nbdkit/tree/block-status -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programm...

...-530,17 +518,11 @@ > return 0; > > REPLY.STRUCTURED_REPLY.FINISH: > - struct command *cmd = h->reply_cmd; > uint16_t flags; > > flags = be16toh (h->sbuf.sr.structured_reply.flags); > - if (flags & NBD_REPLY_FLAG_DONE) { > - if (cmd->type == NBD_CMD_BLOCK_STATUS) > - FREE_CALLBACK (cmd->cb.fn.extent); > - if (cmd->type == NBD_CMD_READ) > - FREE_CALLBACK (cmd->cb.fn.chunk); > + if (flags & NBD_REPLY_FLAG_DONE) > SET_NEXT_STATE (%^FINISH_COMMAND); > - } > else { > h->reply_cmd = NULL; >...

None are supported at present, so this always returns an empty list. --- docs/nbdkit-protocol.pod | 4 ++ server/protocol.h | 2 + server/connections.c | 84 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+) diff --git a/docs/nbdkit-protocol.pod b/docs/nbdkit-protocol.pod index 68438fa..4c65e00 100644 --- a/docs/nbdkit-protocol.pod +++

...nt, uint64_t offset, > > uint32_t flags /* always 0? */, > > Unless we support the REQ_ONE flag (where the client specifically wants > only one extent returned). ... > > nbdkit would need to do some massaging on this to get it into the > > right format for NBD_CMD_BLOCK_STATUS. (I'm very confused about what > > NBD_CMD_FLAG_REQ_ONE is supposed to do.) > > REQ_ONE says that a successful answer has to be exactly one extent, > covering only the head of the region. (That is, force the server to > behave as if obtaining information beyond the first ext...