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 8, 2023, 11:16 AM Niklas Cassel wrote:
> From: Niklas Cassel
>
> Hello John,
>
> This series should be applied on top of the series called:
> "[PATCH v2 0/8] misc AHCI cleanups"
> which can be found here:
> https://lists.nongnu.org/archive/html/qemu-block/2023-06/msg00038.html
>
>
From: Niklas Cassel
Improve ahci_port_check_error() to also assert that PxIS.TFES is set when
expecting errors.
Signed-off-by: Niklas Cassel
---
tests/qtest/libqos/ahci.c | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/tests/qtest/libqos/ahci.c
From: Niklas Cassel
When the HBA encouters an error, the host has to perform error recovery,
see AHCI 1.3.1 section 6.2.2.1, in order to be able issue new commands.
If we don't do this, all the commands that we queue will get aborted.
Some tests, e.g. test_atapi_tray() call
From: Niklas Cassel
Modify ahci_port_check_error() to simply take a struct AHCICommand.
This way, the conditionals are in line which the existing code,
e.g. ahci_port_check_nonbusy(), which checks for cmd->errors.
This makes the code easier to reason with, we don't want to use
cmd->errors in
From: Niklas Cassel
A command that has ERR_STAT set, does not get to clear PxCI.
See AHCI 1.3.1, section 5.3.8, states RegFIS:Entry and RegFIS:ClearCI,
and 5.3.16.5 ERR:FatalTaskfile.
Some tests, e.g. test_atapi_tray() call ahci_atapi_test_ready() with
ready == false, intentionally sending a
From: Niklas Cassel
Hello John,
This series should be applied on top of the series called:
"[PATCH v2 0/8] misc AHCI cleanups"
which can be found here:
https://lists.nongnu.org/archive/html/qemu-block/2023-06/msg00038.html
This series improves the ahci test suite to be in line with
the AHCI
From: Niklas Cassel
A command that has ERR_STAT set, does not get to clear PxCI.
See AHCI 1.3.1, section 5.3.8, states RegFIS:Entry and RegFIS:ClearCI,
and 5.3.16.5 ERR:FatalTaskfile.
Some tests, e.g. test_atapi_tray() call ahci_atapi_test_ready() with
ready == false, intentionally sending a
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
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
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
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
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
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
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
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'
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
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
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
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
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
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
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
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.
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
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.
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
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
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
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
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)
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 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 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;
>>>
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 Mon, 5 Jun 2023 at 16:48, Hanna Czenczek wrote:
>
> When processing vectored guest requests that are not aligned to the
> storage request alignment, we pad them by adding head and/or tail
> buffers for a read-modify-write cycle.
Hi; Coverity complains (CID 1512819) that the assert added
in
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
59 matches
Mail list logo
