search for: nbd_poll

...ers, sending SIGINT to nbdkit was sometimes enough to kill the process (if it hadn't yet read the NBD_CMD_READ, and therefore did try to wait for any outstanding requests before quitting), but often it did not (because nbdkit was stuck waiting for pthread_join()). Then there's the race that nbd_poll() can sometimes get lucky enough to catch a POLLHUP in REPLY.START where recv() returning 0 transitions things to CLOSED, but more often catches a POLLERR and transitions to DEAD. Then there was the hour I spent scratching my head why kill didn't seem to get rid of the child process even thoug...

Rich recently patched things to generate one man page per function rather than libnbd-api.3 (nice), but in doing so got stumped by a problem with a fresh git clone (automake fails for any 'include' directive that does not already exist). I've figured out how to hack around it, but the hack requires GNU make. We already use GNU make constructs elsewhere (such as $(wildcard)), but

While working on a new test of what happens when the server goes away while commands are in flight, I managed to hit a race where I hit death from SIGPIPE instead of a clean transition to the DEAD state. I also found myself wanting to use nbd_poll from the test, but with a way to distinguish between the state machine progressing vs. hanging. Eric Blake (2): socket: Avoid SIGPIPE where possible poll: Improve our interface generator/generator | 18 +++++++++++++----- lib/poll.c | 13 ++++++++++--- lib/socket.c | 7 +++++...

...: "); + + /* Poll while there is no fd. */ + if (nbd_aio_get_fd (nbd) != -1) { + fprintf (stderr, "%s: test failed: " + "nbd_aio_get_fd did not fail prior to connection\n", + argv[0]); + } + check (EINVAL, "nbd_aio_get_fd: "); + if (nbd_poll (nbd, 1000) != -1) { + fprintf (stderr, "%s: test failed: " + "nbd_poll did not fail prior to connection\n", + argv[0]); + } + check (EINVAL, "nbd_poll: "); + + /* Connect to a working server, then try to connect again. */ + if (nbd_conne...

Reverts commit 387cbe67c3db27e8a61117fedb6e7fad76e409ef. --- generator/generator | 18 +++++++++++++++++- lib/handle.c | 28 +++++++++++++++++++++++++++- tests/closure-lifetimes.c | 4 +++- 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/generator/generator b/generator/generator index 2cd83f1..25e4aa5 100755 --- a/generator/generator +++ b/generator/generator

On 6/25/19 8:14 AM, Richard W.M. Jones wrote: > On Tue, Jun 25, 2019 at 02:58:34PM +0200, Martin Kletzander wrote: >> Here are few things I found out when using libnbd that might be perfectly fine >> or maybe just an oversight, but I wanted to point them out. It's nothing major. >> >> When running a program with `nbdkit -U - --run ...`, the $nbd parameter gets

On 6/26/19 11:29 PM, Eric Blake wrote: > Make nbd_poll slightly more like poll(), allowing a user to detect > timeouts by returning 0 on timeout and 1 when we made progress. It > turns out that none of our internal uses ever expect a timeout (we > only call nbd_internal_poll with timeout==-1 because we expect a reply > from the server), but...

Make nbd_poll slightly more like poll(), allowing a user to detect timeouts by returning 0 on timeout and 1 when we made progress. It turns out that none of our internal uses ever expect a timeout (we only call nbd_internal_poll with timeout==-1 because we expect a reply from the server), but the public function...

...that you have to call > nbd_add_meta_context(h, "base:allocation") is also awkward; if we add > convenience macros for the results, we probably also want to support > nbd_add_meta_context(h, LIBNBD_CONTEXT_BASE_ALLOCATION). Still needs to be done. Another thing I just noticed: nbd_poll returns 0 both for timeout and for when it notified the state machine about POLLIN or POLLOUT making progress. We probably want to switch it to return 0 on timeout and 1 when it made progress, so that a user can detect timeout (all our internal callers pass -1 for the timeout parameter, and thus ca...

One API change, some small documentation changes.

...nbd_is_rotational \ - nbd_can_trim \ - nbd_can_zero \ - nbd_can_df \ - nbd_can_multi_conn \ - nbd_can_cache \ - nbd_can_meta_context \ - nbd_get_size \ - nbd_pread \ - nbd_pread_structured \ - nbd_pwrite \ - nbd_shutdown \ - nbd_flush \ - nbd_trim \ - nbd_cache \ - nbd_zero \ - nbd_block_status \ - nbd_poll \ - nbd_aio_connect \ - nbd_aio_connect_uri \ - nbd_aio_connect_unix \ - nbd_aio_connect_tcp \ - nbd_aio_connect_command \ - nbd_aio_pread \ - nbd_aio_pread_structured \ - nbd_aio_pwrite \ - nbd_aio_disconnect \ - nbd_aio_flush \ - nbd_aio_trim \ - nbd_aio_cache \ - nbd_aio_zero \ - nbd_aio_block_s...

...nbd_is_rotational \ + nbd_can_trim \ + nbd_can_zero \ + nbd_can_df \ + nbd_can_multi_conn \ + nbd_can_cache \ + nbd_can_meta_context \ + nbd_get_size \ + nbd_pread \ + nbd_pread_structured \ + nbd_pwrite \ + nbd_shutdown \ + nbd_flush \ + nbd_trim \ + nbd_cache \ + nbd_zero \ + nbd_block_status \ + nbd_poll \ + nbd_aio_connect \ + nbd_aio_connect_uri \ + nbd_aio_connect_unix \ + nbd_aio_connect_tcp \ + nbd_aio_connect_command \ + nbd_aio_pread \ + nbd_aio_pread_structured \ + nbd_aio_pwrite \ + nbd_aio_disconnect \ + nbd_aio_flush \ + nbd_aio_trim \ + nbd_aio_cache \ + nbd_aio_zero \ + nbd_aio_block_s...

...exit (EXIT_FAILURE); + } + } + + while (nbd_aio_in_flight (nbd) > 0) { + int64_t handle = nbd_aio_peek_command_completed (nbd); + + if (handle == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if (handle == 0) { + if (nbd_poll (nbd, -1) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + } + else + nbd_aio_command_completed (nbd, handle); + } + + if (nbd_shutdown (nbd) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (E...

...0, > + read_cb, NULL, > + completion_cb, NULL, 0); > + assert (read_cb_free == 0); > + assert (completion_cb_free == 0); > + while (!nbd_aio_command_completed (nbd, cookie)) > + assert (nbd_poll (nbd, -1) >= 0); More side effects in an assert :( > + > + assert (read_cb_valid == 1); > + assert (completion_cb_valid == 1); > + assert (read_cb_free == 1); > + assert (completion_cb_free == 1); > + > + nbd_close (nbd); > + > + /* Test command callbacks are f...

I think I've addressed everything that was raised in review. Some of the highlights: - Callbacks should be freed reliably along all exit paths. - There's a simple test of closure lifetimes. - I've tried to use VALID|FREE in all the places where I'm confident that it's safe and correct to do. There may be more places. Note this is an optimization and shouldn't

...completion_cb, &completion_cb, 0); if (cookie == -1) NBD_ERROR; - assert (read_cb_free == 0); - assert (completion_cb_free == 0); + assert (read_cb_freed == 0); + assert (completion_cb_freed == 0); while (!nbd_aio_command_completed (nbd, cookie)) { if (nbd_poll (nbd, -1) == -1) NBD_ERROR; } - assert (read_cb_valid == 1); - assert (completion_cb_valid == 1); - assert (read_cb_free == 1); - assert (completion_cb_free == 1); + assert (read_cb_called == 1); + assert (completion_cb_called == 1); + assert (read_cb_freed == 1); + assert (completion_...

...= completion_cb_free }, 0); if (cookie == -1) NBD_ERROR; - assert (read_cb_free == 0); - assert (completion_cb_free == 0); + assert (read_cb_freed == 0); + assert (completion_cb_freed == 0); while (!nbd_aio_command_completed (nbd, cookie)) { if (nbd_poll (nbd, -1) == -1) NBD_ERROR; } - assert (read_cb_valid == 1); - assert (completion_cb_valid == 1); - assert (read_cb_free == 1); - assert (completion_cb_free == 1); + assert (read_cb_called == 1); + assert (completion_cb_called == 1); + assert (read_cb_freed == 1); + assert (completion_...

...llback (nbd, buf, sizeof buf, 0, + read_cb, NULL, + completion_cb, NULL, 0); + assert (read_cb_free == 0); + assert (completion_cb_free == 0); + while (!nbd_aio_command_completed (nbd, cookie)) + assert (nbd_poll (nbd, -1) >= 0); + + assert (read_cb_valid == 1); + assert (completion_cb_valid == 1); + assert (read_cb_free == 1); + assert (completion_cb_free == 1); + + nbd_close (nbd); + + /* Test command callbacks are freed if the handle is closed without + * running the commands. + * + * Note...

As proposed here: https://www.redhat.com/archives/libguestfs/2019-August/msg00130.html I didn't actually read Eric's replies to that yet because I've been concentrating on writing these patches all day. Anyway here they are and I'll look at what Eric said about the proposal next. Rich.

Patches 1 & 2 are rather complex, but the end result is that we pass closures + user_data + free function in single struct parameters as I described previously in this email: https://www.redhat.com/archives/libguestfs/2019-August/msg00210.html Patch 3 adds a convenient FREE_CALLBACK macro which seems a worthwhile simplification if you buy into 1 & 2. Patch 4 adds another macro which is