Re: [Libguestfs] [libnbd PATCH v4 4/4] internal: Refactor layout of replies in sbuf

2023-06-12 Thread Eric Blake
On Thu, Jun 08, 2023 at 09:17:38PM -0500, Eric Blake wrote: > 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

Re: [Libguestfs] [PATCH v4 09/24] nbd: Replace bool structured_reply with mode enum

2023-06-12 Thread Eric Blake
On Mon, Jun 12, 2023 at 06:07:59PM +0300, Vladimir Sementsov-Ogievskiy wrote: > On 08.06.23 16:56, Eric Blake wrote: > > 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

Re: [Libguestfs] [PATCH libnbd 0/2] Two simple patches

2023-06-12 Thread Eric Blake
On Mon, Jun 12, 2023 at 07:27:51PM +0100, Richard W.M. Jones wrote: > These patches aren't related to each other, but both are quite simple. > > The second one requires particular attention - it's my experience that > printing out the state transitions in debug mode has never helped me > to

Re: [Libguestfs] [libnbd PATCH v3 05/22] states: Prepare to receive 64-bit replies

2023-06-12 Thread Eric Blake
On Thu, Jun 01, 2023 at 08:00:55AM -0500, Eric Blake wrote: > > > @@ -83,13 +96,18 @@ REPLY.STRUCTURED_REPLY.CHECK: > > > * not worth keeping the connection alive. > > > */ > > >if (length > MAX_REQUEST_SIZE + sizeof > > > h->sbuf.reply.payload.offset_data) { > > > -set_error (0,

Re: [Libguestfs] [PATCH v4 06/24] nbd/client: Simplify cookie vs. index computation

2023-06-12 Thread Eric Blake
On Mon, Jun 12, 2023 at 05:27:19PM +0300, Vladimir Sementsov-Ogievskiy wrote: > On 08.06.23 16:56, Eric Blake wrote: > > 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

Re: [Libguestfs] [PATCH libnbd 1/2] info: Avoid calling nbd_opt_abort if not in option negotiation mode

2023-06-12 Thread Eric Blake
On Mon, Jun 12, 2023 at 07:27:52PM +0100, Richard W.M. Jones wrote: > This change avoids the following harmless but annoying warning on the > normal exit path: > > libnbd: debug: nbd1: nbd_opt_abort: enter: > libnbd: debug: nbd1: nbd_opt_abort: leave: error="nbd_opt_abort: invalid > state:

Re: [Libguestfs] [libnbd PATCH v4 3/4] generator: Rename states-reply-structured to states-reply-chunk

2023-06-12 Thread Eric Blake
On Thu, Jun 08, 2023 at 09:17:37PM -0500, Eric Blake wrote: > 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

Re: [Libguestfs] [PATCH libnbd 2/2] generator: state machine: Be less verbose in debug messages

2023-06-12 Thread Eric Blake
On Mon, Jun 12, 2023 at 07:27:53PM +0100, Richard W.M. Jones wrote: > Logging state transitions in debug mode produces huge amounts of > output which is not especially helpful. This change removes this > debugging output. This reduces the debug output by approximately two > thirds. > --- >

Re: [Libguestfs] [PATCH v4 08/24] nbd: Use enum for various negotiation modes

2023-06-12 Thread Vladimir Sementsov-Ogievskiy
On 08.06.23 16:56, Eric Blake wrote: 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,

Re: [Libguestfs] [PATCH v4 10/24] nbd/client: Pass mode through to nbd_send_request

2023-06-12 Thread Vladimir Sementsov-Ogievskiy
On 08.06.23 16:56, Eric Blake wrote: 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

Re: [Libguestfs] [PATCH v4 03/24] nbd/server: Prepare for alternate-size headers

2023-06-12 Thread Vladimir Sementsov-Ogievskiy
On 08.06.23 16:56, Eric Blake wrote: 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

Re: [Libguestfs] [PATCH v4 05/24] nbd: s/handle/cookie/ to match NBD spec

2023-06-12 Thread Vladimir Sementsov-Ogievskiy
On 08.06.23 16:56, 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 the nown 'handle' as a way to

Re: [Libguestfs] [PATCH v4 06/24] nbd/client: Simplify cookie vs. index computation

2023-06-12 Thread Vladimir Sementsov-Ogievskiy
On 08.06.23 16:56, Eric Blake wrote: 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

Re: [Libguestfs] [PATCH v4 09/24] nbd: Replace bool structured_reply with mode enum

2023-06-12 Thread Vladimir Sementsov-Ogievskiy
On 08.06.23 16:56, Eric Blake wrote: 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

Re: [Libguestfs] [PATCH v4 02/24] nbd: Consistent typedef usage in header

2023-06-12 Thread Vladimir Sementsov-Ogievskiy
On 08.06.23 17:17, Eric Blake wrote: 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

Re: [Libguestfs] [PATCH v4 11/24] nbd: Add types for extended headers

2023-06-12 Thread Vladimir Sementsov-Ogievskiy
On 08.06.23 16:56, Eric Blake wrote: 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

Re: [Libguestfs] [libnbd PATCH v4 1/4] states: Document our reliance on type overlaps

2023-06-12 Thread Richard W.M. Jones
On Fri, Jun 09, 2023 at 03:39:19PM -0500, Eric Blake wrote: > [Bah - I typed up a longer response, but lost it when accidentally > trying to send through the wrong SMTP server, so now I have to > remember what I had...] > > On Fri, Jun 09, 2023 at 02:45:56PM +0200, Laszlo Ersek wrote: > > On

[Libguestfs] [PATCH libnbd 0/2] Two simple patches

2023-06-12 Thread Richard W.M. Jones
These patches aren't related to each other, but both are quite simple. The second one requires particular attention - it's my experience that printing out the state transitions in debug mode has never helped me to diagnose a bug, but it has made the debug logs huge and hard to follow. However

[Libguestfs] [PATCH libnbd 1/2] info: Avoid calling nbd_opt_abort if not in option negotiation mode

2023-06-12 Thread Richard W.M. Jones
This change avoids the following harmless but annoying warning on the normal exit path: libnbd: debug: nbd1: nbd_opt_abort: enter: libnbd: debug: nbd1: nbd_opt_abort: leave: error="nbd_opt_abort: invalid state: READY: the handle must be negotiating: Invalid argument" --- info/main.c | 7 +--

[Libguestfs] [PATCH libnbd 2/2] generator: state machine: Be less verbose in debug messages

2023-06-12 Thread Richard W.M. Jones
Logging state transitions in debug mode produces huge amounts of output which is not especially helpful. This change removes this debugging output. This reduces the debug output by approximately two thirds. --- generator/state_machine_generator.ml | 12 1 file changed, 12