search for: nbd_rep_meta_context

...i); diff --git a/server/protocol.h b/server/protocol.h index 06b917e..a7de2f0 100644 --- a/server/protocol.h +++ b/server/protocol.h @@ -112,6 +112,7 @@ extern const char *name_of_nbd_rep (int); #define NBD_REP_ACK 1 #define NBD_REP_SERVER 2 #define NBD_REP_INFO 3 +#define NBD_REP_META_CONTEXT 4 #define NBD_REP_ERR_UNSUP 0x80000001 #define NBD_REP_ERR_POLICY 0x80000002 #define NBD_REP_ERR_INVALID 0x80000003 @@ -128,6 +129,18 @@ struct fixed_new_option_reply_info_export { uint16_t eflags; /* per-export flags */ } __attribute__((packed)); +/* NBD_REP_META_CONTEX...

...i); diff --git a/server/protocol.h b/server/protocol.h index 4fe3c75..a7de2f0 100644 --- a/server/protocol.h +++ b/server/protocol.h @@ -112,6 +112,7 @@ extern const char *name_of_nbd_rep (int); #define NBD_REP_ACK 1 #define NBD_REP_SERVER 2 #define NBD_REP_INFO 3 +#define NBD_REP_META_CONTEXT 4 #define NBD_REP_ERR_UNSUP 0x80000001 #define NBD_REP_ERR_POLICY 0x80000002 #define NBD_REP_ERR_INVALID 0x80000003 @@ -128,6 +129,18 @@ struct fixed_new_option_reply_info_export { uint16_t eflags; /* per-export flags */ } __attribute__((packed)); +/* NBD_REP_META_CONTEX...

...-1; - } switch (reply) { case NBD_REP_ACK: /* End of list of replies. */ - if (len != 0) { - SET_NEXT_STATE (%.DEAD); - set_error (0, "handshake: invalid option reply length"); - return -1; - } SET_NEXT_STATE (%^OPT_GO.START); break; case NBD_REP_META_CONTEXT: /* A context. */ if (len > maxpayload) debug (h, "skipping too large meta context"); - else if (len <= sizeof h->sbuf.or.payload.context.context) { - /* A valid reply has at least one byte in payload.context.str */ - set_error (0, "handshake: NBD_R...

I'm still playing with ideas on how to split rstate from wstate (so that we can send a request without waiting for POLLIN to complete a pending reply), but this is some preliminary refactoring I found useful. I also fixed a couple of bugs while in the area (already pushed). There's a question of whether we want nbd_handle to be nearly 5k, or if we should instead keep it small and add one

...col.h +++ b/lib/nbd-protocol.h @@ -123,15 +123,19 @@ struct nbd_fixed_new_option_reply { #define NBD_REP_ERR(val) (0x80000000 | (val)) #define NBD_REP_IS_ERR(val) (!!((val) & 0x80000000)) -#define NBD_REP_ACK 1 -#define NBD_REP_SERVER 2 -#define NBD_REP_INFO 3 -#define NBD_REP_META_CONTEXT 4 -#define NBD_REP_ERR_UNSUP NBD_REP_ERR (1) -#define NBD_REP_ERR_POLICY NBD_REP_ERR (2) -#define NBD_REP_ERR_INVALID NBD_REP_ERR (3) -#define NBD_REP_ERR_PLATFORM NBD_REP_ERR (4) -#define NBD_REP_ERR_TLS_REQD NBD_REP_ERR (5) +#define NBD_REP_ACK 1 +#define NBD_REP_SERVER...

...eply; /* NBD_REP_* */ @@ -110,33 +119,37 @@ struct fixed_new_option_reply { #define NBD_REP_ERR(val) (0x80000000 | (val)) #define NBD_REP_IS_ERR(val) (!!((val) & 0x80000000)) -#define NBD_REP_ACK 1 -#define NBD_REP_SERVER 2 -#define NBD_REP_INFO 3 -#define NBD_REP_META_CONTEXT 4 -#define NBD_REP_ERR_UNSUP NBD_REP_ERR (1) -#define NBD_REP_ERR_POLICY NBD_REP_ERR (2) -#define NBD_REP_ERR_INVALID NBD_REP_ERR (3) -#define NBD_REP_ERR_PLATFORM NBD_REP_ERR (4) -#define NBD_REP_ERR_TLS_REQD NBD_REP_ERR (5) +#define NBD_REP_ACK 1 +#define NBD_REP_SERVER...

...s. */ - SET_NEXT_STATE (%^OPT_GO.START); + if (opt == NBD_OPT_LIST_META_CONTEXT) { + SET_NEXT_STATE (%.NEGOTIATING); + CALL_CALLBACK (h->opt_cb.completion, &err); + nbd_internal_free_option (h); + } + else + SET_NEXT_STATE (%^OPT_GO.START); break; case NBD_REP_META_CONTEXT: /* A context. */ if (len > maxpayload) @@ -194,21 +225,38 @@ STATE_MACHINE { } debug (h, "negotiated %s with context ID %" PRIu32, meta_context->name, meta_context->context_id); - meta_context->next = h->meta_contexts; - h->me...

...a9) @@ -140,19 +146,19 @@ struct nbd_fixed_new_option_reply_info_export { uint16_t info; /* NBD_INFO_EXPORT */ uint64_t exportsize; /* size of export */ uint16_t eflags; /* per-export flags */ -} __attribute__((packed)); +} NBD_ATTRIBUTE_PACKED; /* NBD_REP_META_CONTEXT reply (follows fixed_new_option_reply). */ struct nbd_fixed_new_option_reply_meta_context { uint32_t context_id; /* metadata context ID */ /* followed by a string */ -} __attribute__((packed)); +} NBD_ATTRIBUTE_PACKED; /* NBD_REPLY_TYPE_BLOCK_STATUS block descriptor. */ struct n...

We should have only one NBD protocol file. Let's make nbdkit's version the canonical one, and use it in libnbd. Rich.

I'm posting this now, as I'm at the end of a workday and I got things working for manual experimentation. Still to do: - write interop tests for qemu-nbd and nbdkit (including my proposed patch addition of qemu-nbd -A to show qemu:allocation-depth) - figure out if we can make 'nbdinfo --map' use the new API to automatically select all contexts advertised by the server Eric Blake

...t; 0) @@ -146,19 +146,19 @@ struct nbd_fixed_new_option_reply_info_export { uint16_t info; /* NBD_INFO_EXPORT */ uint64_t exportsize; /* size of export */ uint16_t eflags; /* per-export flags */ -} __attribute__((packed)); +} NBD_ATTRIBUTE_PACKED; /* NBD_REP_META_CONTEXT reply (follows fixed_new_option_reply). */ struct nbd_fixed_new_option_reply_meta_context { uint32_t context_id; /* metadata context ID */ /* followed by a string */ -} __attribute__((packed)); +} NBD_ATTRIBUTE_PACKED; /* NBD_REPLY_TYPE_BLOCK_STATUS block descriptor. */ struct n...

Proposal for new APIs to list exports. The general shape of the API can probably best be seen from the examples/list-exports.c example. Rich.

...@@ negotiate_handshake_newstyle_options (struct connection *conn) conn->meta_context_base_allocation = false; if (nr_queries == 0) { if (option == NBD_OPT_LIST_META_CONTEXT) { - if (send_newstyle_option_reply_meta_context - (conn, option, NBD_REP_META_CONTEXT, - 0, "base:allocation") == -1) + if (send_newstyle_option_reply_meta_context (option, + NBD_REP_META_CONTEXT, + 0, "base:allocation") +...

Not sure what version we're up to, but this reimplements extents using the new simpler structure described in this thread: https://www.redhat.com/archives/libguestfs/2019-March/msg00077.html I also fixed most of the things that Eric pointed out in the previous review, although I need to go back over his replies and check I've got everything. This needs a bit more testing. However the

v1 was here: https://www.redhat.com/archives/libguestfs/2020-February/msg00081.html v2 replaces struct connection *conn = GET_CONN; with GET_CONN; which sets conn implicitly and asserts that it is non-NULL. If we actually want to test if conn is non-NULL or behave differently, then you must use threadlocal_get_conn() instead, and some existing uses do that. Rich.

On Wed, Feb 22, 2023 at 11:49:18AM +0200, Wouter Verhelst wrote: > On Mon, Nov 14, 2022 at 04:46:52PM -0600, Eric Blake wrote: > [...] > > @@ -1370,9 +1475,10 @@ of the newstyle negotiation. > > Return a list of `NBD_REP_META_CONTEXT` replies, one per context, > > followed by an `NBD_REP_ACK` or an error. > > > > - This option SHOULD NOT be requested unless structured replies have > > - been negotiated first. If a client attempts to do so, a server > > - MAY send `NBD_REP_ERR_INVALID...

I'm not sure exactly which version we're up to, but let's say it's version 4. I'm a lot happier with this version: - all filters have been reviewed and changed where I think that's necessary - can_extents is properly defined and implemented now - NBD protocol is followed - I believe it addresses all previous review points where possible The "only" thing

The third patch is a large but mechanical change which gets rid of passing around struct connection * entirely within the server, preferring instead to reference the connection through thread-local storage. I hope this is a gateway to simplifying other parts of the code. Rich.

...it { + uint16_t info; /* NBD_INFO_INIT_STATE */ + uint16_t flags; /* per-export init flags */ +} NBD_ATTRIBUTE_PACKED; + +/* Constants for use in reply to NBD_INFO_INIT_STATE. */ +#define NBD_INIT_SPARSE (1 << 0) +#define NBD_INIT_ZERO (1 << 1) + /* NBD_REP_META_CONTEXT reply (follows fixed_new_option_reply). */ struct nbd_fixed_new_option_reply_meta_context { uint32_t context_id; /* metadata context ID */ -- 2.24.1

Some of these affect attempts to connect to older qemu-nbd versions, some of them were triggered by manual edits to qemu-nbd source code to provoke various other compliant (if uncommon) server behaviors. Eric Blake (4): starttls: Skip error payload if falling back to unencrypted states: Reject payload to NBD_REP_ACK meta-context: Skip error payload if server lacks meta_context states: Add