search for: nbd_reply_type_offset_hole

Rather than requiring all servers and clients to have a 32-bit limit on maximum NBD_CMD_READ/WRITE sizes, we can choose to standardize support for a 64-bit single I/O transaction now. NBD_REPLY_TYPE_OFFSET_DATA can already handle a large reply, but NBD_REPLY_TYPE_OFFSET_HOLE needs a 64-bit counterpart. By standardizing this, all clients must be prepared to support both types of hole type replies, even though most server implementations of extended replies are likely to only send one hole type. --- As this may mean a corner-case that gets less testing, I have separat...

...cs/nbdkit-protocol.pod index a9a3390..f706cfd 100644 --- a/docs/nbdkit-protocol.pod +++ b/docs/nbdkit-protocol.pod @@ -139,6 +139,19 @@ Supported in nbdkit E<ge> 1.11.10. Only C<base:allocation> (ie. querying which parts of an image are sparse) is supported. +Sparse reads (using C<NBD_REPLY_TYPE_OFFSET_HOLE> are not directly +supported, but a client can use block status to infer which portions +of the export do not need to be read. + +=item C<NBD_FLAG_DF> + +Supported in nbdkit E<ge> 1.11.11. + +This protocol extension allows a client to force an all-or-none read +when structured replie...

...0..f706cfd 100644 > --- a/docs/nbdkit-protocol.pod > +++ b/docs/nbdkit-protocol.pod > @@ -139,6 +139,19 @@ Supported in nbdkit E<ge> 1.11.10. > Only C<base:allocation> (ie. querying which parts of an image are > sparse) is supported. > > +Sparse reads (using C<NBD_REPLY_TYPE_OFFSET_HOLE> are not directly > +supported, but a client can use block status to infer which portions > +of the export do not need to be read. > + > +=item C<NBD_FLAG_DF> > + > +Supported in nbdkit E<ge> 1.11.11. > + > +This protocol extension allows a client to force an...

...f callbacks, where a > different callback is invoked for different chunk types: > > set = nbd_callback_set_create(nbd, NBD_CMD_READ, opaque, default_cb, > default_error_cb); > nbd_callback_set_add(nbd, set, NBD_REPLY_TYPE_OFFSET_DATA, opaque, cb); > nbd_callback_set_add(nbd, set, NBD_REPLY_TYPE_OFFSET_HOLE, opaque, cb); > nbd_pread_callback(nbd, buf, count, offset, set, flags); This would also work, with the caveat that the Python bindings will currently crash if you try to do this. I'm trying to fix that now. > The idea of registering a set of callbacks to handle a particular > integ...

On Mon, Jun 17, 2019 at 07:07:53PM -0500, Eric Blake wrote: > The NBD spec requires that a server doing structured reads must not > succeed unless it covers the entire buffer with reply chunks. In the > general case, this requires a lot of bookkeeping to check whether > offsets were non-overlapping and sufficient, and we'd rather defer > such checking to an optional callback

On Mon, May 27, 2019 at 09:01:01PM -0500, Eric Blake wrote: > diff --git a/lib/rw.c b/lib/rw.c > index feaf468..343c340 100644 > --- a/lib/rw.c > +++ b/lib/rw.c > @@ -234,11 +234,17 @@ int64_t > nbd_unlocked_aio_pread (struct nbd_handle *h, void *buf, > size_t count, uint64_t offset, uint32_t flags) > { > - if (flags != 0) { > + if ((flags

.../* Structured reply flags. */ +extern const 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...

...--- a/docs/nbdkit-protocol.pod >> +++ b/docs/nbdkit-protocol.pod >> @@ -139,6 +139,19 @@ Supported in nbdkit E<ge> 1.11.10. >> Only C<base:allocation> (ie. querying which parts of an image are >> sparse) is supported. >> >> +Sparse reads (using C<NBD_REPLY_TYPE_OFFSET_HOLE> are not directly >> +supported, but a client can use block status to infer which portions >> +of the export do not need to be read. >> + >> +=item C<NBD_FLAG_DF> >> + >> +Supported in nbdkit E<ge> 1.11.11. >> + >> +This protocol extens...

...at's a larger diffstat, and thus at odds 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 mag...

...OCK_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_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 ge...

...let the user register a set of callbacks, where a different callback is invoked for different chunk types: set = nbd_callback_set_create(nbd, NBD_CMD_READ, opaque, default_cb, default_error_cb); nbd_callback_set_add(nbd, set, NBD_REPLY_TYPE_OFFSET_DATA, opaque, cb); nbd_callback_set_add(nbd, set, NBD_REPLY_TYPE_OFFSET_HOLE, opaque, cb); nbd_pread_callback(nbd, buf, count, offset, set, flags); The idea of registering a set of callbacks to handle a particular integer command id may work well for other extensions as well; particularly if we encounter a case where an app wants to use libnbd for the groundwork (setting u...

..._DATA too small"); > + free (buf); > + return nbd_mark_dead (h); > + } > + memcpy (&offset, buf, sizeof offset); > + offset = be64toh (offset); > + len = rep.structured.length - sizeof offset; > + break; leaks buf > + case NBD_REPLY_TYPE_OFFSET_HOLE: > + if (rep.structured.length != sizeof offset + sizeof len) { > + nbdkit_error ("structured reply OFFSET_HOLE size incorrect"); > + free (buf); > + return nbd_mark_dead (h); > + } > + memcpy (&offset, buf, sizeof offset); > +...

...client, | | trace_nbd_co_send_structured_read_hole(handle, offset + progress, | pnum); | - set_be_chunk(&chunk.h, final ? NBD_REPLY_FLAG_DONE : 0, | + set_be_chunk(&chunk.h, 0, | NBD_REPLY_TYPE_OFFSET_HOLE, | handle, sizeof(chunk) - sizeof(chunk.h)); | stq_be_p(&chunk.offset, offset + progress); | stl_be_p(&chunk.length, pnum); | ret = nbd_co_send_iov(client, iov, 1, errp); | + if (ret == 0) { | + NBDSt...

...0) #define NBD_REPLY_TYPE_ERR(val) ((1<<15) | (val)) #define NBD_REPLY_TYPE_IS_ERR(val) (!!((val) & (1<<15))) /* 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 @@ -213,7 +206,6 @@ extern const char *name_of_nbd_reply_type (int); #define NBD_REPLY_TYPE_ERROR_OFFSET NBD_REPLY_TYPE_ERR (2) /* NBD commands. */ -extern const char *name_of_nbd_cmd (int); #define NBD_CMD_READ 0 #define NBD_CMD_WRITE 1 #define NBD_CMD_DISC...

...ge 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/6:[0062] [FC] 'RFC: spec: Introduce NBD_REPLY_TYPE_OFFSET_HOLE_EXT' Eric Blake (6): spec: Recommend cap on NBD_REPLY_TYPE_BLOCK_STATUS length spec: Change maximum block size to maximum payload size spec: Add NBD_OPT_EXTENDED_HEADERS spec: Allow 64-bit block status results spec: Introduce NBD_FLAG_BLOCK_STATUS_PAYLOAD RFC: spec: Introduce NBD_R...

I'm hoping to implement .extents for the nbd plugin; this is a prerequisite. I'm not sure about patch 3 - if we like it, I'll squash it to 2, if we don't, I think we are okay just dropping it. I'm also wondering if we have to worry about malicious plugins that don't populate the entire .pread buffer in an effort to get nbdkit to expose portions of the heap; my patch 7 loses

...+ nbdkit_error ("structured reply OFFSET_DATA too small"); + free (buf); + return nbd_mark_dead (h); + } + memcpy (&offset, buf, sizeof offset); + offset = be64toh (offset); + len = rep.structured.length - sizeof offset; + break; + case NBD_REPLY_TYPE_OFFSET_HOLE: + if (rep.structured.length != sizeof offset + sizeof len) { + nbdkit_error ("structured reply OFFSET_HOLE size incorrect"); + free (buf); + return nbd_mark_dead (h); + } + memcpy (&offset, buf, sizeof offset); + offset = be64toh (offset); +...

...client, | | trace_nbd_co_send_structured_read_hole(handle, offset + progress, | pnum); | - set_be_chunk(&chunk.h, final ? NBD_REPLY_FLAG_DONE : 0, | + set_be_chunk(&chunk.h, 0, | NBD_REPLY_TYPE_OFFSET_HOLE, | handle, sizeof(chunk) - sizeof(chunk.h)); | stq_be_p(&chunk.offset, offset + progress); | stl_be_p(&chunk.length, pnum); | ret = nbd_co_send_iov(client, iov, 1, errp); | + if (ret == 0) { | + NBDSt...

...ute__((packed)); + /* New-style handshake server reply when using NBD_OPT_EXPORT_NAME. * Modern clients use NBD_OPT_GO instead of this. */ @@ -187,7 +200,7 @@ 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_BLOCK_STATUS 5 #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...

...ents_can_sr, .can_extents = noextents_can_extents, }; diff --git a/TODO b/TODO index 332400b3..87621791 100644 --- a/TODO +++ b/TODO @@ -198,4 +198,5 @@ using ‘#define NBDKIT_API_VERSION <version>’. that supports .extents, the v2 protocol would allow us to at least synthesize NBD_REPLY_TYPE_OFFSET_HOLE for less network traffic, even though the plugin will still have to fully populate the .pread - buffer; the v3 protocol should make sparse reads more direct. + buffer; the v3 protocol should make sparse reads more direct. Also, + the filter callback .can_sr may make sense for a v3 plugin....