Upcoming patches to add extended headers want to share the common
payload parser with structured replies. Renaming the file and the
associated states from "structured" to "chunk" makes it more obvious
that we will be sharing the code independent from the header style
parsed in the earlier REPLY
In order to more easily add a third reply type with an even larger
header, but where the payload will look the same for both structured
and extended replies, it is nicer if simple and structured replies are
nested inside the same layer of sbuf.reply.hdr. Doing this also lets
us add an alias for
When I added structured replies to the NBD spec, I intentionally chose
a wire layout where the magic number and cookie overlap, even while
the middle member changes from uint32_t error to the pair uint16_t
flags and type. Based only on a strict reading of C rules on
effective types and compatible
This was v3 patch 2/22, reworked to address the confusion about how a
structured reply header is read in two pieces before getting to the
payload portion.
I'm still working on rebasing the rest of my v3 series (patches 1,
3-22) from other comments given, but this seemed independent enough
that
Splitting the parse of a 20-byte structured reply header across two
source files (16 bytes overlapping with simple replies in
states-reply.c, the remaining 4 bytes in states-reply-structured.c) is
confusing. The upcoming addition of extended headers will reuse the
payload parsing portion of
When reading a diff of files used to generate other code, it helps to
see API changes up front, and to see state machine changes in the
order in which states are generally encountered, while leaving
language binding changes for last.
Signed-off-by: Eric Blake
---
Based on an idea here:
On Thu, Jun 08, 2023 at 08:56:53AM -0500, Eric Blake wrote:
> Allow a client to request a subset of negotiated meta contexts. For
> example, a client may ask to use a single connection to learn about
> both block status and dirty bitmaps, but where the dirty bitmap
> queries only need to be
On Thu, Jun 08, 2023 at 08:56:52AM -0500, Eric Blake wrote:
> The next commit will add support for the optional extension
> NBD_CMD_FLAG_PAYLOAD during NBD_CMD_BLOCK_STATUS, where the client can
> request that the server only return a subset of negotiated contexts,
> rather than all contexts. To
On Thu, Jun 08, 2023 at 08:56:47AM -0500, Eric Blake wrote:
> Instead of ignoring the low-level error just to refabricate our own
> message to pass to the caller, we can just plump the caller's errp
plumb
(at least I got it right in the subject line...)
> down to the low level.
>
>
On Thu, Jun 08, 2023 at 08:56:42AM -0500, Eric Blake wrote:
> Upcoming additions to support NBD 64-bit effect lengths allow for the
> possibility to distinguish between payload length (capped at 32M) and
> effect length (up to 63 bits). Without that extension, only the
Technically, the NBD spec
On Thu, Jun 08, 2023 at 08:56:41AM -0500, Eric Blake wrote:
> Widen the length field of NBDRequest to 64-bits, although we can
> assert that all current uses are still under 32 bits, because nothing
> ever puts us into NBD_MODE_EXTENDED yet. Thus no semantic change. No
> semantic change yet.
I
On Thu, Jun 08, 2023 at 08:56:34AM -0500, Eric Blake wrote:
> Externally, libnbd exposed the 64-bit opaque marker for each client
> NBD packet as the "cookie", because it was less confusing when
> contrasted with 'struct nbd_handle *' holding all libnbd state. It
> also avoids confusion between
On Thu, Jun 08, 2023 at 08:56:31AM -0500, Eric Blake wrote:
> We had a mix of struct declarataions followed by typedefs, and direct
declarations
> struct definitions as part of a typedef. Pick a single style. Also
> float a couple of opaque typedefs earlier in the file, as a later
> patch
Peform several minor refactorings of how the list of negotiated meta
contexts is managed, to make upcoming patches easier: Promote the
internal type NBDExportMetaContexts to the public opaque type
NBDMetaContexts, and mark exp const. Use a shorter member name in
NBDClient. Hoist calls to
Allow a client to request a subset of negotiated meta contexts. For
example, a client may ask to use a single connection to learn about
both block status and dirty bitmaps, but where the dirty bitmap
queries only need to be performed on a subset of the disk; forcing the
server to compute that
Update the client code to be able to send an extended request, and
parse an extended header from the server. Note that since we reject
any structured reply with a too-large payload, we can always normalize
a valid header back into the compact form, so that the caller need not
deal with two
All the pieces are in place for a client to finally request extended
headers. Note that we must not request extended headers when qemu-nbd
is used to connect to the kernel module (as nbd.ko does not expect
them, but expects us to do the negotiation in userspace before handing
the socket over to
Once extended mode is enabled, we need to accept 64-bit status replies
(even for replies that don't exceed a 32-bit length). It is easier to
normalize narrow replies into wide format so that the rest of our code
only has to handle one width. Although a server is non-compliant if
it sends a
Although extended mode is not yet enabled, once we do turn it on, we
need to accept extended requests for all messages. Previous patches
have already taken care of supporting 64-bit lengths, now we just need
to read it off the wire.
Note that this implementation will block indefinitely on a
The next commit will add support for the optional extension
NBD_CMD_FLAG_PAYLOAD during NBD_CMD_BLOCK_STATUS, where the client can
request that the server only return a subset of negotiated contexts,
rather than all contexts. To make that task easier, this patch
populates the list of contexts to
Time to start supporting clients that request extended headers. Now
we can finally reach the code added across several previous patches.
Even though the NBD spec has been altered to allow us to accept
NBD_CMD_READ larger than the max payload size (provided our response
is a hole or broken up
The NBD spec states that if the client negotiates extended headers,
the server must avoid NBD_REPLY_TYPE_BLOCK_STATUS and instead use
NBD_REPLY_TYPE_BLOCK_STATUS_EXT which supports 64-bit lengths, even if
the reply does not need more than 32 bits. As of this patch,
client->mode is still never
Instead of ignoring the low-level error just to refabricate our own
message to pass to the caller, we can just plump the caller's errp
down to the low level.
Signed-off-by: Eric Blake
---
v4: new patch [Vladimir]
---
block/nbd.c | 16 ++--
1 file changed, 10 insertions(+), 6
Although extended mode is not yet enabled, once we do turn it on, we
need to reply with extended headers to all messages. Update the low
level entry points necessary so that all other callers automatically
get the right header based on the current mode.
Signed-off-by: Eric Blake
---
v4: new
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 upstream nbd.git (through commit
e6f3b94a934). This patch does not change any existing behavior, but
merely sets the stage for
Upcoming additions to support NBD 64-bit effect lengths allow for the
possibility to distinguish between payload length (capped at 32M) and
effect length (up to 63 bits). Without that extension, only the
NBD_CMD_WRITE request has a payload; but with the extension, it makes
sense to allow at least
Widen the length field of NBDRequest to 64-bits, although we can
assert that all current uses are still under 32 bits, because nothing
ever puts us into NBD_MODE_EXTENDED yet. Thus no semantic change. No
semantic change yet.
Signed-off-by: Eric Blake
---
v4: split off enum changes to earlier
The upcoming patches for 64-bit extensions requires various points in
the protocol to make decisions based on what was negotiated. While we
could easily add a 'bool extended_headers' alongside the existing
'bool structured_reply', this does not scale well if more modes are
added in the future.
Deciphering the hard-coded list of integer return values from
nbd_start_negotiate() will only get more confusing when adding support
for 64-bit extended headers. Better is to name things in an enum.
Although the function in question is private to client.c, putting the
enum in a public header and
Once the 64-bit headers extension is enabled, the data layout we send
over the wire for a client request depends on the mode negotiated with
the server. Rather than adding a parameter to nbd_send_request, we
can add a member to struct NBDRequest, since it already does not
reflect on-wire format.
Our existing use of structured replies either reads into a qiov capped
at 32M (NBD_CMD_READ) or caps allocation to 1000 bytes (see
NBD_MAX_MALLOC_PAYLOAD in block/nbd.c). But the existing length
checks are rather late; if we encounter a buggy (or malicious) server
that sends a super-large payload
Externally, libnbd exposed the 64-bit opaque marker for each client
NBD packet as the "cookie", because it was less confusing when
contrasted with 'struct nbd_handle *' holding all libnbd state. It
also avoids confusion between the nown 'handle' as a way to identify a
packet and the verb 'handle'
Upstream NBD now documents[1] an extension that supports 64-bit effect
lengths in requests. As part of that extension, the size of the reply
headers will change in order to permit a 64-bit length in the reply
for symmetry[2]. Additionally, where the reply header is currently 16
bytes for simple
Our code relies on a sentinel cookie value of zero for deciding when a
packet has been handled, as well as relying on array indices between 0
and MAX_NBD_REQUESTS-1 for dereferencing purposes. As long as we can
symmetrically convert between two forms, there is no reason to go with
the odd choice
Part of NBD's 64-bit headers extension involves passing the client's
requested offset back as part of the reply header (one reason it
stated for this change: converting absolute offsets stored in
NBD_REPLY_TYPE_OFFSET_DATA to relative offsets within the buffer is
easier if the absolute offset of
We had a mix of struct declarataions followed by typedefs, and direct
struct definitions as part of a typedef. Pick a single style. Also
float a couple of opaque typedefs earlier in the file, as a later
patch wants to refer NBDExport* in NBDRequest. No semantic impact.
Signed-off-by: Eric
v3 was here:
https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg03607.html
Since then, I've incorporated lots of feedback from Vladimir:
- split several patches into smaller pieces
- use an enum to track various negotiation modes
- reorder a few patches
- several new patches (2, 5, 6)
Assigning strlen() to a uint32_t and then asserting that it isn't too
large doesn't catch the case of an input string 4G in length.
Thankfully, the incoming strings can never be that large: if the
export name or query is reflecting a string the client got from the
server, we already guarantee that
On Thu, Jun 08, 2023 at 02:38:40PM +0200, Laszlo Ersek wrote:
> On 6/8/23 14:20, Richard W.M. Jones wrote:
> > On Thu, Jun 08, 2023 at 01:48:41PM +0200, Laszlo Ersek wrote:
> >> On 6/7/23 12:00, Richard W.M. Jones wrote:
> >>> On Tue, May 30, 2023 at 05:48:25PM +0200, Laszlo Ersek wrote:
>
On Wed, Jun 07, 2023 at 04:23:27PM +0200, Laszlo Ersek wrote:
[...]
> > diff --git a/ocaml/helpers.c b/ocaml/helpers.c
> > index 3361a696..09666daf 100644
> > --- a/ocaml/helpers.c
> > +++ b/ocaml/helpers.c
> > @@ -133,6 +133,26 @@ nbd_internal_ocaml_alloc_i64_from_u32_array (uint32_t
> > *a,
On Thu, Jun 08, 2023 at 10:37:59AM +0100, Richard W.M. Jones wrote:
> Yes, the API is nicer now we return the subelements as a list instead
> of having to iterate over the list in pairs. I might change that to
> an array or struct after these patches go upstream as that will be a
> tiny bit more
On 6/8/23 14:20, Richard W.M. Jones wrote:
> On Thu, Jun 08, 2023 at 01:48:41PM +0200, Laszlo Ersek wrote:
>> On 6/7/23 12:00, Richard W.M. Jones wrote:
>>> On Tue, May 30, 2023 at 05:48:25PM +0200, Laszlo Ersek wrote:
BTW I'm foreseeing a problem: if the extended block descriptor can
On Wed, Jun 07, 2023 at 07:18:46PM +0100, Richard W.M. Jones wrote:
> I don't want to actually link to them to avoid giving them link-karma,
> but the old repositories / now mirrors at:
>
> github.com/libguestfs/libnbd
> github.com/libguestfs/nbdkit
>
> stopped mirroring the true repositories:
>
On Thu, Jun 08, 2023 at 01:48:41PM +0200, Laszlo Ersek wrote:
> On 6/7/23 12:00, Richard W.M. Jones wrote:
> > On Tue, May 30, 2023 at 05:48:25PM +0200, Laszlo Ersek wrote:
> >> BTW I'm foreseeing a problem: if the extended block descriptor can
> >> provide an unsigned 64-bit length, we're going
On 6/7/23 20:18, Richard W.M. Jones wrote:
> I don't want to actually link to them to avoid giving them link-karma,
> but the old repositories / now mirrors at:
>
> github.com/libguestfs/libnbd
> github.com/libguestfs/nbdkit
>
> stopped mirroring the true repositories:
>
>
On 6/7/23 14:59, Richard W.M. Jones wrote:
> On Tue, Jun 06, 2023 at 08:06:50PM +0100, Richard W.M. Jones wrote:
>>
>> Michael Henriksen pointed out an issue with this approach.
>>
>> If the web server is actually generating the content on the fly then
>> it may send it as chunked encoding, and in
On 6/6/23 21:06, Richard W.M. Jones wrote:
>
> Michael Henriksen pointed out an issue with this approach.
>
> If the web server is actually generating the content on the fly then
> it may send it as chunked encoding, and in HTTP/1.1 it's not required
> that the Content-Length field is present
On 6/7/23 12:00, Richard W.M. Jones wrote:
> On Tue, May 30, 2023 at 05:48:25PM +0200, Laszlo Ersek wrote:
>> BTW I'm foreseeing a problem: if the extended block descriptor can
>> provide an unsigned 64-bit length, we're going to have trouble exposing
>> that in OCaml, because OCaml only has
On 6/7/23 16:55, Richard W.M. Jones wrote:
> On Thu, Jun 01, 2023 at 11:04:05AM +0200, Laszlo Ersek wrote:
>> On 5/25/23 15:00, Eric Blake wrote:
>>> @@ -69,11 +75,18 @@ REPLY.STRUCTURED_REPLY.RECV_REMAINING:
>>> REPLY.STRUCTURED_REPLY.CHECK:
>>>struct command *cmd = h->reply_cmd;
>>>
On 6/6/23 18:38, Richard W.M. Jones wrote:
> On Tue, Jun 06, 2023 at 06:09:09PM +0200, Laszlo Ersek wrote:
>> On 6/6/23 13:22, Richard W.M. Jones wrote:
>>> @@ -250,6 +255,11 @@ handle_requests (int s)
>>>}
>>>memcpy (path, [5], n);
>>>path[n] = '\0';
>>> + if
From: Ravi Singh
---
appliance/hostfiles.in| 4 ++
appliance/init| 2 +
daemon/Makefile.am| 7 +++
daemon/guestfsd.c | 3 +
daemon/listfs.ml | 17 ++
daemon/vm.ml | 68 ++
daemon/vm.mli | 24
Reposting this patch as a patch, original message here:
https://listman.redhat.com/archives/libguestfs/2023-June/031722.html
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Sorry it took me so long to get around to this one. Hopefully
the next version will need only cursory approval.
I did read all of the patches, and I basically agree with Laszlo on
the ones he reviewed already. Therefore I didn't add any comment
except where necessary. You can assume Acked-by
On Thu, May 25, 2023 at 08:01:08AM -0500, Eric Blake wrote:
> As part of extending NBD to support 64-bit lengths, the protocol also
> added an option for servers to allow clients to request filtered
> responses to NBD_CMD_BLOCK_STATUS when more than one meta-context is
> negotiated (see NBD commit
On Thu, May 25, 2023 at 08:01:07AM -0500, Eric Blake wrote:
> In the recent NBD protocol extensions to add 64-bit commands [1], an
> additional option was added to allow NBD_CMD_BLOCK_STATUS pass a
to pass
> client payload instructing the server to filter its answers in nbd.git
> commit e6f3b94a
On Thu, May 25, 2023 at 08:01:06AM -0500, Eric Blake wrote:
> Prove that we can round-trip a block status request larger than 4G
> through a new-enough qemu-nbd. Also serves as a unit test of our shim
> for converting internal 64-bit representation back to the older 32-bit
> nbd_block_status
On Thu, May 25, 2023 at 08:01:05AM -0500, Eric Blake wrote:
> Very similar to the recent addition of nbd_opt_structured_reply,
> giving us fine-grained control over an extended headers request.
>
> Because nbdkit does not yet support extended headers, testsuite
> coverage is limited to interop
On Thu, May 25, 2023 at 08:01:04AM -0500, Eric Blake wrote:
> This is the culmination of the previous patches' preparation work for
> using extended headers when possible. The new states in the state
> machine are copied extensively from our handling of
> OPT_STRUCTURED_REPLY. The next patch
On Thu, May 25, 2023 at 08:01:03AM -0500, Eric Blake wrote:
> Since our example program for 32-bit extents is inherently limited to
> 32-bit lengths, it is also worth demonstrating the 64-bit extent API,
> including the difference in the array indexing being saner.
>
> Signed-off-by: Eric Blake
On Thu, May 25, 2023 at 08:01:02AM -0500, Eric Blake wrote:
> Although our use of "base:allocation" doesn't require the use of the
> 64-bit API for flags, we might perform slightly faster for a server
> that does give us 64-bit extent lengths and honors larger nbd_zero
> lengths.
>
>
On Thu, May 25, 2023 at 08:01:01AM -0500, Eric Blake wrote:
> Although we usually map "base:allocation" which doesn't require the
> use of the 64-bit API for flags, this application IS intended to map
> out other metacontexts that might have 64-bit flags. And when
> extended headers are in use,
On Thu, May 25, 2023 at 08:01:00AM -0500, Eric Blake wrote:
> Add another bit of overall server information, as well as a '--can
> extended-headers' silent query. For now, the testsuite is written
> assuming that when nbdkit finally adds extended headers support, it
> will also add a --no-eh kill
On Thu, May 25, 2023 at 08:00:59AM -0500, Eric Blake wrote:
> Although our use of "base:allocation" doesn't require the use of the
> 64-bit API for flags, we might perform slightly faster for a server
> that does give us 64-bit extent lengths.
>
> Signed-off-by: Eric Blake
> ---
> dump/dump.c |
On Thu, May 25, 2023 at 08:00:58AM -0500, Eric Blake wrote:
> Although our use of "base:allocation" doesn't require the use of the
> 64-bit API for flags, we might perform slightly faster for a server
> that does give us 64-bit extent lengths and honors larger nbd_zero
> lengths.
I think this
64 matches
Mail list logo