search for: meta_context_base_alloc

...| if (!conn->structured_replies) { | nbdkit_error ("invalid request: " | "%s: structured replies was not negotiated", | @@ -536,7 +536,7 @@ send_structured_reply_block_status (uint64_t cookie, | size_t i; | int r; | | - assert (conn->meta_context_base_allocation); | + // assert (conn->meta_context_base_allocation); | assert (cmd == NBD_CMD_BLOCK_STATUS); | | blocks = extents_to_block_descriptors (extents, flags, count, offset, plus this sequence: $ patched/nbdkit memory 1M $ ./run nbdsh --opt-mode -u nbd://localhost nbd> h.set_request_...

...files changed, 229 insertions(+), 15 deletions(-) diff --git a/server/internal.h b/server/internal.h index ae51804..58d5794 100644 --- a/server/internal.h +++ b/server/internal.h @@ -183,6 +183,7 @@ struct connection { bool can_multi_conn; bool using_tls; bool structured_replies; + bool meta_context_base_allocation; int sockin, sockout; connection_recv_function recv; @@ -219,6 +220,12 @@ extern int protocol_handshake_newstyle (struct connection *conn) extern int protocol_recv_request_send_reply (struct connection *conn) __attribute__((__nonnull__ (1))); +/* The context ID of base:allocation...

...* save it in the connection. + * save it in the connection. For NBD_OPT_GO, if an earlier + * NBD_OPT_SET_META_CONTEXT used an export name, it must match + * or else we drop the support for that context. */ + if (option == NBD_OPT_GO && conn->meta_context_base_allocation && + (exportnamelen != conn->exportnamelen || + memcmp (conn->exportname, &data[4], exportnamelen) != 0)) + conn->meta_context_base_allocation = false; free (conn->exportname); conn->exportname = strndup (&data[4]...

...files changed, 335 insertions(+), 16 deletions(-) diff --git a/server/internal.h b/server/internal.h index 825dd3e..03d6119 100644 --- a/server/internal.h +++ b/server/internal.h @@ -183,6 +183,7 @@ struct connection { bool can_multi_conn; bool using_tls; bool structured_replies; + bool meta_context_base_allocation; int sockin, sockout; connection_recv_function recv; @@ -219,6 +220,12 @@ extern int protocol_handshake_newstyle (struct connection *conn) extern int protocol_recv_request_send_reply (struct connection *conn) __attribute__((__nonnull__ (1))); +/* The context ID of base:allocation...

...lies) { > | nbdkit_error ("invalid request: " > | "%s: structured replies was not negotiated", > | @@ -536,7 +536,7 @@ send_structured_reply_block_status (uint64_t cookie, > | size_t i; > | int r; > | > | - assert (conn->meta_context_base_allocation); > | + // assert (conn->meta_context_base_allocation); > | assert (cmd == NBD_CMD_BLOCK_STATUS); > | > | blocks = extents_to_block_descriptors (extents, flags, count, offset, > > plus this sequence: > > $ patched/nbdkit memory 1M > $ ./run nbdsh --opt-m...

The first one is the nastiest - it is an assertion failure caused by a spec-compliant client and introduced by our security fix that was released in 1.14.1. Eric Blake (4): server: Fix regression for NBD_OPT_INFO before NBD_OPT_GO server: Fix back-to-back SET_META_CONTEXT server: Forbid NUL in export and context names server: Fix OPT_GO on different export than SET_META_CONTEXT

...NOSPC : EINVAL; + if (!conn->structured_replies) { + nbdkit_error ("invalid request: " + "%s: structured replies was not negotiated", + name_of_nbd_cmd (cmd)); + *error = EINVAL; + return false; + } + if (!conn->meta_context_base_allocation) { + nbdkit_error ("invalid request: " + "%s: base:allocation was not negotiated", + name_of_nbd_cmd (cmd)); + *error = EINVAL; return false; } break; @@ -144,79 +137,6 @@ validate_request (struct connection *...

...@@ -239,8 +239,12 @@ check_export_name (struct connection *conn, uint32_t option, char *buf, assert (exportnamelen < sizeof conn->exportname); if (save) { + if (exportnamelen != conn->exportnamelen || + memcmp (conn->exportname, buf, exportnamelen) != 0) + conn->meta_context_base_allocation = false; memcpy (conn->exportname, buf, exportnamelen); conn->exportname[exportnamelen] = '\0'; + conn->exportnamelen = exportnamelen; } debug ("newstyle negotiation: %s: client requested export '%.*s'", name_of_nbd_opt (option),...

...RE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock); > + struct structured_reply reply; > + size_t nr_extents = nbdkit_extents_size (extents); > + uint32_t context_id; > + size_t i; > + int r; > + > + assert (cmd == NBD_CMD_BLOCK_STATUS); > + Worth asserting conn->meta_context_base_allocation? > + > + /* Send each block descriptor. */ > + for (i = 0; i < nr_extents; ++i) { Where does the list terminate after 1 extent if REQ_ONE was set? Also, if REQ_ONE is set but the plugin provided coalesced status beyond the request, this would need to clamp the answer to the re...

...struct handle *handles; /* One per plugin and filter. */ size_t nr_handles; - char exportname[NBD_MAX_STRING + 1]; - uint32_t exportnamelen; uint32_t cflags; uint16_t eflags; bool handshake_complete; @@ -255,6 +253,9 @@ struct connection { bool structured_replies; bool meta_context_base_allocation; + char *exportname_from_set_meta_context; + char *exportname; + int sockin, sockout; connection_recv_function recv; connection_send_function send; @@ -273,8 +274,9 @@ extern int connection_set_status (int value); /* protocol-handshake.c */ extern int protocol_handshake (void)...

...ter. */ > size_t nr_handles; > > - char exportname[NBD_MAX_STRING + 1]; > - uint32_t exportnamelen; > uint32_t cflags; > uint16_t eflags; > bool handshake_complete; > @@ -255,6 +253,9 @@ struct connection { > bool structured_replies; > bool meta_context_base_allocation; > > + char *exportname_from_set_meta_context; > + char *exportname; Interesting switch from array to pointer, but it should work. > +++ b/server/plugins.c > @@ -278,12 +278,30 @@ plugin_preconnect (struct backend *b, int readonly) > } > > static void * &g...

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

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

...ver/internal.h @@ -237,6 +237,7 @@ reset_handle (struct handle *h) h->can_cache = -1; } +DEFINE_VECTOR_TYPE(string_vector, char *); struct connection { pthread_mutex_t request_lock; pthread_mutex_t read_lock; @@ -258,8 +259,9 @@ struct connection { bool structured_replies; bool meta_context_base_allocation; + string_vector interns; char *exportname_from_set_meta_context; - char *exportname; + const char *exportname; int sockin, sockout; connection_recv_function recv; @@ -298,6 +300,9 @@ extern int protocol_recv_request_send_reply (void); */ #define base_allocation_id 1 +/* publ...

...ver/internal.h @@ -237,6 +237,7 @@ reset_handle (struct handle *h) h->can_cache = -1; } +DEFINE_VECTOR_TYPE(string_vector, char *); struct connection { pthread_mutex_t request_lock; pthread_mutex_t read_lock; @@ -258,8 +259,9 @@ struct connection { bool structured_replies; bool meta_context_base_allocation; + string_vector interns; char *exportname_from_set_meta_context; - char *exportname; + const char *exportname; int sockin, sockout; connection_recv_function recv; @@ -298,6 +300,9 @@ extern int protocol_recv_request_send_reply (void); */ #define base_allocation_id 1 +/* publ...

...n { uint32_t cflags; uint16_t eflags; - bool readonly; - bool can_flush; - bool is_rotational; - bool can_trim; - bool can_zero; - bool can_fua; - bool can_multi_conn; - bool can_cache; - bool emulate_cache; - bool can_extents; bool using_tls; bool structured_replies; bool meta_context_base_allocation; diff --git a/server/backend.c b/server/backend.c index 374d8540..196b48e4 100644 --- a/server/backend.c +++ b/server/backend.c @@ -144,73 +144,130 @@ backend_get_size (struct backend *b, struct connection *conn) int backend_can_write (struct backend *b, struct connection *conn) { + struct...

I want to post this but mainly for discussion and early review. It's not safe for these patches to all go upstream yet (because not all filters have been checked/adjusted), but if any patches were to go upstream then probably 1 & 2 only are safe. File, VDDK, memory and data plugins all work, although I have only done minimal testing on them. The current tests, such as they are, all

This has already been pushed upstream. I am simply posting these here so we have a reference in the mailing list in case we find bugs later (as I'm sure we will - it's a complex patch series). Great thanks to Eric Blake for tireless review on this one. It also seems to have identified a few minor bugs in qemu along the way. Rich.

Since the v1 series (0/4, at [1]), I've applied patches 1 and 2, rewritten patch 3 [Forbid NUL in export and context names] into patch 4 here, patch 4 there turned into patch 6 here, and everything else here is new. [1]https://www.redhat.com/archives/libguestfs/2019-September/msg00180.html I don't know if there is a handy reusable function for checking whether a string contains valid

It's easy to use the sh script to demonstrate that nbdkit is inefficiently calling into .get_size, .can_fua, and friends more than necessary. We've also commented on the list in the past that it would be nice to ensure that when filters call into next_ops, they are not violating constraints (as we've have to fix several bugs in the past where we did not have such checking to protect