search for: nbd_internal_hexdump

...K) + return 1; /* more data */ + /* sock->ops->recv called set_error already. */ + return -1; + } + if (r == 0) { + set_error (0, "recv: server disconnected unexpectedly"); + return -1; + } #ifdef DUMP_PACKETS - if (h->rbuf != NULL) - nbd_internal_hexdump (h->rbuf, r, stderr); + if (h->rbuf != NULL) + nbd_internal_hexdump (h->rbuf, r, stderr); #endif - if (h->rbuf) - h->rbuf += r; - h->rlen -= r; - if (h->rlen == 0) - return 0; /* move to next state */ - else - return 1;...

I thought this should produce a fairly dramatic performance gain. In fact I couldn't measure any performance difference at all. I think what's happening is we're actually paying an extra syscall (to discover the socket would block) and then doing the poll anyway. So I don't know if it's worth having this patch. It could be argued that it makes the code shorter (therefore

...nbd_internal_fork_safe_perror (h->argv[0]); + if (errno == ENOENT) + _exit (127); + else + _exit (126); } /* Parent. */ diff --git a/lib/internal.h b/lib/internal.h index bdb0e83..31bc3d4 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -384,5 +384,7 @@ extern void nbd_internal_hexdump (const void *data, size_t len, FILE *fp); extern size_t nbd_internal_string_list_length (char **argv); extern char **nbd_internal_copy_string_list (char **argv); extern void nbd_internal_free_string_list (char **argv); +extern const char *nbd_internal_fork_safe_itoa (long v, char *buf, size_t le...

...SET_NEXT_STATE (%RECV_STRUCTURED_REMAINING); > } > else { > @@ -145,8 +148,8 @@ REPLY.CHECK_REPLY_MAGIC: > SET_NEXT_STATE (%.DEAD); > set_error (0, "invalid reply magic 0x%" PRIx32, magic); > #if 0 /* uncomment to see desynchronized data */ > - nbd_internal_hexdump (&h->sbuf.simple_reply, > - sizeof (h->sbuf.simple_reply), > + nbd_internal_hexdump (&h->sbuf.reply.hdr.simple, > + sizeof (h->sbuf.reply.hdr.simple), > stderr); > #endif > re...

...tate state); extern enum state_group nbd_internal_state_group (enum state state); extern enum state_group nbd_internal_state_group_parent (enum state_group group); +#define set_state(h,next_state) ((h)->state) = (next_state) +#define get_state(h) ((h)->state) + /* utils.c */ extern void nbd_internal_hexdump (const void *data, size_t len, FILE *fp); extern size_t nbd_internal_string_list_length (char **argv); diff --git a/lib/is-state.c b/lib/is-state.c index 55d103b..51a2d47 100644 --- a/lib/is-state.c +++ b/lib/is-state.c @@ -104,40 +104,40 @@ nbd_internal_is_state_closed (enum state state) int nb...

...; -#define set_state(h,next_state) ((h)->state) = (next_state) -#define get_state(h) ((h)->state) +#define set_next_state(h,next_state) ((h)->state) = (next_state) +#define get_next_state(h) ((h)->state) +#define get_public_state(h) ((h)->public_state) /* utils.c */ extern void nbd_internal_hexdump (const void *data, size_t len, FILE *fp); diff --git a/lib/is-state.c b/lib/is-state.c index c941ab4..b2c20df 100644 --- a/lib/is-state.c +++ b/lib/is-state.c @@ -98,44 +98,48 @@ nbd_internal_is_state_closed (enum state state) return state == STATE_CLOSED; } -/* NB: is_locked = false, may_set...

...; -#define set_state(h,next_state) ((h)->state) = (next_state) -#define get_state(h) ((h)->state) +#define set_next_state(h,next_state) ((h)->state) = (next_state) +#define get_next_state(h) ((h)->state) +#define get_public_state(h) ((h)->public_state) /* utils.c */ extern void nbd_internal_hexdump (const void *data, size_t len, FILE *fp); diff --git a/lib/is-state.c b/lib/is-state.c index c941ab4..b2c20df 100644 --- a/lib/is-state.c +++ b/lib/is-state.c @@ -98,44 +98,48 @@ nbd_internal_is_state_closed (enum state state) return state == STATE_CLOSED; } -/* NB: is_locked = false, may_set...

This was v3 patch 2/22, reworked to address the confusion about how a structured reply header is read in two pieces before getting to the payload portion. I'm still working on rebasing the rest of my v3 series (patches 1, 3-22) from other comments given, but this seemed independent enough that it's worth posting now rather than holding it up for the rest of the series. Eric Blake (4):

v1 was here: https://www.redhat.com/archives/libguestfs/2019-June/thread.html#00055 v2 was here: https://www.redhat.com/archives/libguestfs/2019-June/thread.html#00067 v3: - Fix atomicly -> atomically in commit message. - Fix a comment. - Fix TOCTTOU: There is now an inline function generated called <name>_is_permitted_state, and this is called twice, first outside the

Patch 1/6 was posted before and I didn't change it: https://www.redhat.com/archives/libguestfs/2019-May/thread.html#00134 That doesn't necessarily mean I shouldn't change it, I'm posting it again because the other patches depend on it. The main change in this series is we add three new API functions: nbd_aio_is_created - connection has just been created

This adds coalescing of adjacent extents of the same type, as mentioned by Eric Blake in the commit message here: https://github.com/libguestfs/libnbd/commit/46072f6611f80245846a445766da071e457b00cd The patch series is rather long because it detours through adding the <vector.h> library from nbdkit into libnbd and replacing ad hoc uses of realloc, char ** etc in various places. Rich.

v1 was posted here: https://www.redhat.com/archives/libguestfs/2019-September/thread.html#00337 v2: - Drop the first patch. - Hopefully fix the multiple issues with fork-safety and general behaviour on error paths. Note this requires execvpe for which there seems to be no equivalent on FreeBSD, except some kind of tedious path parsing (but can we assign to environ?) Rich.

I need to think about this patch series a bit more, but it does at least pass the tests. Rich.

This isn't quite finished because not all of the tests or examples have been updated, but it demonstrates an idea: Should we forget about the concept of having multiple connections managed under a single handle? In this patch there is a single ‘struct nbd_handle *’ which manages a single state machine and connection (and therefore no nbd_connection). To connect to a multi-conn server you must