search for: querylen

...shake_newstyle_options (struct connection *conn) conn->structured_replies = true; break; + case NBD_OPT_LIST_META_CONTEXT: + case NBD_OPT_SET_META_CONTEXT: + { + uint32_t opt_index; + uint32_t exportnamelen; + uint32_t nr_queries; + uint32_t querylen; + const char *what; + + optname = name_of_nbd_opt (option); + if (conn_recv_full (conn, data, optlen, "read: %s: %m", optname) == -1) + return -1; + + if (!conn->structured_replies) { + if (send_newstyle_option_reply (conn, option, NBD_REP...

...nd_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1) return -1; } @@ -525,7 +575,34 @@ negotiate_handshake_newstyle_options (struct connection *conn) option == NBD_OPT_LIST_META_CONTEXT ? "query" : "set", (int) querylen, &data[opt_index]); - /* Ignore query - nothing is supported. */ + /* For LIST, "base:" returns all supported contexts in the + * base namespace. We only support "base:allocation". + */ + if (option == NBD_OPT_LIST_...

...ption, NBD_REP_ACK) == -1) - return -1; - - conn->structured_replies = true; - break; - - case NBD_OPT_LIST_META_CONTEXT: - case NBD_OPT_SET_META_CONTEXT: - { - uint32_t opt_index; - uint32_t exportnamelen; - uint32_t nr_queries; - uint32_t querylen; - const char *what; - - optname = name_of_nbd_opt (option); - if (conn_recv_full (conn, data, optlen, "read: %s: %m", optname) == -1) - return -1; - - if (!conn->structured_replies) { - if (send_newstyle_option_reply (conn, option, NBD_REP...

...nd_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1) return -1; } @@ -525,7 +575,32 @@ negotiate_handshake_newstyle_options (struct connection *conn) option == NBD_OPT_LIST_META_CONTEXT ? "query" : "set", (int) querylen, &data[opt_index]); - /* Ignore query - nothing is supported. */ + /* For LIST, "base:" returns all supported contexts in the + * base namespace. We only support "base:allocation". + */ + if (option == NBD_OPT_LIST_...

These are a couple of patches in preparation for the Block Status implementation. While the patches (especially the second one) are very large they are really just elementary code motion. Rich.

Updated based on other changes that have happened in the meantime: - rely more on cleanup.h (throughout) - split structured read for easier review (patch 2 and 3 were combined in v1) - rely on nbdkit not leaking a server's partial answer (patch 3) - add tests (patch 5) - other bug fixes I found while testing it - drop EOVERFLOW patch for now; it will be separate once upstream NBD protocol

...E is still viable, or -1 on inability to connect. */ -static int -nbdplug_newstyle_haggle (struct handle *h) -{ - const char *const query = "base:allocation"; - struct new_option opt; - uint32_t exportnamelen = htobe32 (strlen (export)); - uint32_t nrqueries = htobe32 (1); - uint32_t querylen = htobe32 (strlen (query)); - /* For now, we make no NBD_INFO_* requests, relying on the server to - send its defaults. TODO: nbdkit should let plugins report block - sizes, at which point we should request NBD_INFO_BLOCK_SIZE and - obey any sizes set by server. */ - uint16_t nrinfos...

...E is still viable, or -1 on inability to connect. */ -static int -nbdplug_newstyle_haggle (struct handle *h) -{ - const char *const query = "base:allocation"; - struct new_option opt; - uint32_t exportnamelen = htobe32 (strlen (export)); - uint32_t nrqueries = htobe32 (1); - uint32_t querylen = htobe32 (strlen (query)); - /* For now, we make no NBD_INFO_* requests, relying on the server to - send its defaults. TODO: nbdkit should let plugins report block - sizes, at which point we should request NBD_INFO_BLOCK_SIZE and - obey any sizes set by server. */ - uint16_t nrinfos...

...E is still viable, or -1 on inability to connect. */ -static int -nbd_newstyle_haggle (struct handle *h) -{ - const char *const query = "base:allocation"; - struct nbd_new_option opt; - uint32_t exportnamelen = htobe32 (strlen (export)); - uint32_t nrqueries = htobe32 (1); - uint32_t querylen = htobe32 (strlen (query)); - /* For now, we make no NBD_INFO_* requests, relying on the server to - send its defaults. TODO: nbdkit should let plugins report block - sizes, at which point we should request NBD_INFO_BLOCK_SIZE and - obey any sizes set by server. */ - uint16_t nrinfos...

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

I'm hoping to implement .extents for the nbd plugin; this is a prerequisite. I'm not sure about patch 3 - if we like it, I'll squash it to 2, if we don't, I think we are okay just dropping it. I'm also wondering if we have to worry about malicious plugins that don't populate the entire .pread buffer in an effort to get nbdkit to expose portions of the heap; my patch 7 loses

Patch 1 played with an early draft of Rich's Fedora 30 libnbd package: https://bugzilla.redhat.com/show_bug.cgi?id=1713767#c17 Note that comment 21 provides a newer package 0.1.1-1 with a different API; and that libnbd has more unreleased API changes in the pipeline (whether that will be called 0.2 or 0.1.2); so we'll have to tweak things based on what is actually available in distros.

libnbd-0.1.2-1 is now available in Fedora 29/30 updates-testing, although it was not compiled against libxml2 so it lacks uri support (I ended up testing patch 4 with a self-built libnbd). Diffs since v1 - rebase to master, bump from libnbd 0.1 to 0.1.2, add URI support, better timing results Still not done - patch 5 needs associated tests Eric Blake (5): nbd: Check for libnbd nbd:

libnbd-0.1.4-1 is now available in Fedora 29/30 updates testing. Diffs since v2 - rebase to master, bump from libnbd 0.1.2 to 0.1.3+, add tests to TLS usage which flushed out the need to turn relative pathnames into absolute, doc tweaks Now that the testsuite covers TLS and libnbd has been fixed to provide the things I found lacking when developing v2, I'm leaning towards pushing this on

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

...f (send_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1) + if (send_newstyle_option_reply (option, NBD_REP_ACK) == -1) return -1; conn->structured_replies = true; @@ -569,14 +565,14 @@ negotiate_handshake_newstyle_options (struct connection *conn) uint32_t querylen; const char *what; - if (conn_recv_full (conn, data, optlen, "read: %s: %m", optname) == -1) + if (conn_recv_full (data, optlen, "read: %s: %m", optname) == -1) return -1; /* Note that we support base:allocation whether or not the p...

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.

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

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.