search for: protocol_handshake_newstyle

...> + return -1; > + } ... when I run this patch series under valgrind I get mainly errors originating at this malloc: ==1251605== 58 bytes in 4 blocks are definitely lost in loss record 4 of 10 ==1251605== at 0x896180B: malloc (vg_replace_malloc.c:309) ==1251605== by 0x11909F: protocol_handshake_newstyle (protocol-handshake-news tyle.c:288) ==1251605== by 0x118804: protocol_handshake (protocol-handshake.c:55) ==1251605== by 0x112080: handle_single_connection (connections.c:165) ==1251605== by 0x11B84D: start_thread (sockets.c:276) ==1251605== by 0x8BB74E1: start_thread (pthread_create.c...

...h a maximimum limit on threads? */ break; case 'U': diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c index 486d416f..9801f7c2 100644 --- a/server/protocol-handshake-newstyle.c +++ b/server/protocol-handshake-newstyle.c @@ -673,7 +673,7 @@ protocol_handshake_newstyle (struct connection *conn) struct new_handshake handshake; uint16_t gflags; - gflags = NBD_FLAG_FIXED_NEWSTYLE | NBD_FLAG_NO_ZEROES; + gflags = (NBD_FLAG_FIXED_NEWSTYLE | NBD_FLAG_NO_ZEROES) & mask_handshake; debug ("newstyle negotiation: flags: global 0x%x", gflags); @@...

This is similar to the recent --no-sr option - it's a change that is unlikely to ever be used except by someone testing whether a client is compliant to the protocol, but in that niche case, it can be quite handy (it's a lot nicer to be able to purposefully cripple a server from the command line than from a one-off compile, when testing if a client's fallback for a spec-compliant but

...? offsetof (struct nbd_new_handshake_finish, zeroes) : sizeof handshake_finish, 0) == -1) { nbdkit_error ("write: %s: %m", optname); return -1; @@ -670,7 +670,7 @@ negotiate_handshake_newstyle_options (struct connection *conn) int protocol_handshake_newstyle (struct connection *conn) { - struct new_handshake handshake; + struct nbd_new_handshake handshake; uint16_t gflags; gflags = (NBD_FLAG_FIXED_NEWSTYLE | NBD_FLAG_NO_ZEROES) & mask_handshake; @@ -678,7 +678,7 @@ protocol_handshake_newstyle (struct connection *conn) debug ("new...

...> > ... when I run this patch series under valgrind I get mainly errors > originating at this malloc: > > ==1251605== 58 bytes in 4 blocks are definitely lost in loss record 4 of 10 > ==1251605== at 0x896180B: malloc (vg_replace_malloc.c:309) > ==1251605== by 0x11909F: protocol_handshake_newstyle (protocol-handshake-news > tyle.c:288) > ==1251605== by 0x118804: protocol_handshake (protocol-handshake.c:55) > ==1251605== by 0x112080: handle_single_connection (connections.c:165) > ==1251605== by 0x11B84D: start_thread (sockets.c:276) > ==1251605== by 0x8BB74E1: start...

More nbd-protocol.h improvements Eric Blake (2): common/protocol: Switch nbdmagic to uint64_t common/protocol: Declare additional constants common/protocol/nbd-protocol.h | 16 ++++++++++------ server/protocol-handshake-newstyle.c | 2 +- server/protocol-handshake-oldstyle.c | 2 +- plugins/nbd/nbd-standalone.c | 2 +- tests/test-layers.c | 2 +- 5 files

..., uint16_t *flags) + __attribute__((__nonnull__ (1, 2))); /* protocol-handshake-oldstyle.c */ -extern int protocol_handshake_oldstyle (struct connection *conn) - __attribute__((__nonnull__ (1))); +extern int protocol_handshake_oldstyle (void); /* protocol-handshake-newstyle.c */ -extern int protocol_handshake_newstyle (struct connection *conn) - __attribute__((__nonnull__ (1))); +extern int protocol_handshake_newstyle (void); /* protocol.c */ -extern int protocol_recv_request_send_reply (struct connection *conn) - __attribute__((__nonnull__ (1))); +extern int protocol_recv_request_send_reply (void); /* T...

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.

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.

..."backend_finalize") at assert.c:101 #4 0x00005577156ad04e in backend_finalize (b=<optimized out>, conn=conn at entry=0x55771f1ac710) at backend.c:240 #5 0x00005577156b6ea8 in negotiate_handshake_newstyle_options (conn=<optimized out>) at protocol-handshake-newstyle.c:484 #6 protocol_handshake_newstyle (conn=0x55771f1ac710) at protocol-handshake-newstyle.c:762 #7 0x00005577156b5705 in protocol_handshake (conn=conn at entry=0x55771f1ac710) at protocol-handshake.c:55 #8 0x00005577156af73a in handle_single_connection (sockin=<optimized out>, sockout=15) at connections.c:167 #9 0x00005577156...

...794 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. As far as I can tell it doesn't + * matter what this is as long as nbdkit always returns the same + * number. + */ +#de...

...119 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. As far as I can tell it doesn't + * matter what this is as long as nbdkit always returns the same + * number. + */ +#de...

...protocol_compute_eflags (struct connection *conn, uint16_t *flags) + __attribute__((__nonnull__ (1, 2))); + +/* protocol-handshake-oldstyle.c */ +extern int protocol_handshake_oldstyle (struct connection *conn) + __attribute__((__nonnull__ (1))); + +/* protocol-handshake-newstyle.c */ +extern int protocol_handshake_newstyle (struct connection *conn) + __attribute__((__nonnull__ (1))); + +/* protocol.c */ +extern int protocol_recv_request_send_reply (struct connection *conn) + __attribute__((__nonnull__ (1))); /* crypto.c */ #define root_tls_certificates_dir sysconfdir "/pki/" PACKAGE_NAME diff --git a/...

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

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.

This time around, the numbers are indeed looking better than in v1; and I like the interface better. Eric Blake (2): server: Prefer send() over write() server: Group related transmission send()s server/internal.h | 7 +++- server/connections.c | 51 +++++++++++++++++++++++++--- server/crypto.c | 11 ++++--

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

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

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