search for: nbd_aio_get_fd

...tions(-) diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c index 20c06e2a..8997174e 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -71,7 +71,6 @@ struct transaction { struct handle { /* These fields are read-only once initialized */ struct nbd_handle *nbd; - int fd; /* Cache of nbd_aio_get_fd */ int fds[2]; /* Pipe for kicking the reader thread */ bool readonly; pthread_t reader; @@ -316,7 +315,7 @@ nbdplug_reader (void *handle) while (!nbd_aio_is_dead (h->nbd) && !nbd_aio_is_closed (h->nbd)) { struct pollfd fds[2] = { - [0].fd = h->fd, + [0...

There are several races / deadlocks which I've thought about. Let's see if I can remember them all ... (1) This I experienced: nbd_aio_get_fd deadlocks if there are concurrent synchronous APIs going on. A typical case is where you set up the concurrent writer thread before connecting, and then call a synchronous connect function such as connect_tcp. The synchronous function grabs h->lock, then writes something, which eventually invo...

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

...le == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + goto error; + } + handles[in_flight] = handle; + i--; + in_flight++; + if (in_flight > status->most_in_flight) + status->most_in_flight = in_flight; + } fds[0].fd = nbd_aio_get_fd (nbd); - fds[0].events = want_to_send ? POLLOUT : 0; + fds[0].events = 0; fds[0].revents = 0; dir = nbd_aio_get_direction (nbd); if ((dir & LIBNBD_AIO_DIRECTION_READ) != 0) @@ -266,30 +277,6 @@ start_thread (void *arg) (fds[0].revents & POLLOUT) != 0)...

...uf) != -1) { + fprintf (stderr, "%s: test failed: " + "nbd_set_export_name did not reject large name\n", + argv[0]); + exit (EXIT_FAILURE); + } + check (ENAMETOOLONG, "nbd_set_export_name: "); + + /* 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 fai...

...In the IRC discussion we gave these the preliminary names h->wstate and h->rstate. ---------------------------------------------------------------------- It's worth also saying how the current API works, although we might want to change it. You grab the underlying file descriptor using nbd_aio_get_fd, which is what you poll on. You also have to call nbd_aio_get_direction which returns READ, WRITE or BOTH (== READ|WRITE). You then set up some events mechanism (eg. poll, epoll, etc.), poll until the file descriptor is ready, and call one of nbd_aio_notify_read or nbd_aio_notify_write. The dire...

v1: https://www.redhat.com/archives/libguestfs/2019-June/msg00014.html I pushed a few bits which are uncontroversial. The main changes since v1 are: An extra patch removes the want_to_send / check for nbd_aio_is_ready in examples/threaded-reads-and-writes.c. This logic was wrong since commit 6af72b87 as was pointed out by Eric in his review. Comments and structure of

...for (i = 0; i < MAX_IN_FLIGHT; i++) { + if (handles[i] == 0) { + handles[i] = handle; + break; + } + } + if (nbd_aio_in_flight (nbd) > status->most_in_flight) + status->most_in_flight = nbd_aio_in_flight (nbd); } fds[0].fd = nbd_aio_get_fd (nbd); @@ -291,16 +294,16 @@ start_thread (void *arg) nbd_aio_notify_write (nbd); /* If a command is ready to retire, retire it. */ - for (i = 0; i < in_flight; ++i) { + for (i = 0; i < MAX_IN_FLIGHT; ++i) { + if (handles[i] == 0) + continue; r = nbd_aio_...

...le == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + goto error; + } + handles[in_flight] = handle; + i--; + in_flight++; + if (in_flight > status->most_in_flight) + status->most_in_flight = in_flight; + } + + fds[0].fd = nbd_aio_get_fd (nbd); + fds[0].events = 0; + fds[0].revents = 0; + + dir = nbd_aio_get_direction (nbd); + if ((dir & LIBNBD_AIO_DIRECTION_WRITE) != 0) { + /* The concurrent writer is always writable, we don't have to + * test the socket in poll. Since calling nbd_aio_notify_write +...

...81a79a8cc2d03844cfff5db3ef/lib/socket.c#L32 but for recv the data will come from a buffer populated by the caller using their own thread. The caller will be required to set up a second thread which does basically: pthread_create (start_second_thread); start_second_thread () { int fd = nbd_aio_get_fd (nbd); char buf[BUFSIZ]; for (;;) { poll (fd); len = recv (fd, buf, sizeof buf, 0); nbd_aio_threaded_read (fd, buf, len); } } Note this doesn't require any changes to the state machine, and probably not any changes to the existing main loop. The state machine s...

...preliminary > names h->wstate and h->rstate. > > ---------------------------------------------------------------------- > > It's worth also saying how the current API works, although we might > want to change it. > > You grab the underlying file descriptor using nbd_aio_get_fd, which is > what you poll on. You also have to call nbd_aio_get_direction which > returns READ, WRITE or BOTH (== READ|WRITE). You then set up some > events mechanism (eg. poll, epoll, etc.), poll until the file > descriptor is ready, and call one of nbd_aio_notify_read or > nbd_ai...

...we want to send another request and there's room to issue it + * and the connection is in the READY state so it can be used to + * issue a request. + */ + want_to_send = + i > 0 && in_flight < MAX_IN_FLIGHT && nbd_aio_is_ready (nbd); + + fds[0].fd = nbd_aio_get_fd (nbd); + fds[0].events = want_to_send ? POLLOUT : 0; + fds[0].revents = 0; + + dir = nbd_aio_get_direction (nbd); + if ((dir & LIBNBD_AIO_DIRECTION_WRITE) != 0) { + /* The concurrent writer is always writable, we don't have to + * test the socket in poll. Since calli...

This works, but there's no time saving and I'm still investigating whether it does what I think it does. Nevertheless I thought I would post it because it (probably) implements the idea I had last night outlined in: https://www.redhat.com/archives/libguestfs/2019-June/msg00010.html The meat of the change is patch 4. Patch 5 is an example which I would probably fold into patch 4 for

I fixed the deadlock - turned out to be an actual bug in the nbd plugin (see patch 8). I changed the command syntax so it's now: nbdkit nbd command=qemu arg=-f arg=qcow2 arg=/path/to/disk.qcow2 Nir wrote: 18:08 < nsoffer> rwmjones: regarding the nbd proxy patches, did you have specific flow that help us? 18:08 < nsoffer> rwmjones: or this is just a way to support qcow2 in the

...io_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_status \ - nbd_aio_get_fd \ - nbd_aio_get_direction \ - nbd_aio_notify_read \ - nbd_aio_notify_write \ - nbd_aio_is_created \ - nbd_aio_is_connecting \ - nbd_aio_is_ready \ - nbd_aio_is_processing \ - nbd_aio_is_dead \ - nbd_aio_is_closed \ - nbd_aio_command_completed \ - nbd_aio_peek_command_completed \ - nbd_aio_in_flight...

...io_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_status \ + nbd_aio_get_fd \ + nbd_aio_get_direction \ + nbd_aio_notify_read \ + nbd_aio_notify_write \ + nbd_aio_is_created \ + nbd_aio_is_connecting \ + nbd_aio_is_ready \ + nbd_aio_is_processing \ + nbd_aio_is_dead \ + nbd_aio_is_closed \ + nbd_aio_command_completed \ + nbd_aio_peek_command_completed \ + nbd_aio_in_flight...

...quot;); + in_flight++; + + /* Now wait for commands to retire, or for deadlock to occur */ + while (in_flight > 0) { + if (nbd_aio_is_dead (conn) || nbd_aio_is_closed (conn)) { + fprintf (stderr, "connection is dead or closed\n"); + goto error; + } + + fds[0].fd = nbd_aio_get_fd (conn); + fds[0].events = 0; + fds[0].revents = 0; + dir = nbd_aio_get_direction (conn); + if ((dir & LIBNBD_AIO_DIRECTION_READ) != 0) + fds[0].events |= POLLIN; + if ((dir & LIBNBD_AIO_DIRECTION_WRITE) != 0) + fds[0].events |= POLLOUT; + + if (poll (fds, 1, -1)...

...r; + } + in_flight++; + + /* Now wait for commands to retire, or for deadlock to occur */ + while (in_flight > 0) { + if (nbd_aio_is_dead (conn) || nbd_aio_is_closed (conn)) { + fprintf (stderr, "connection is dead or closed\n"); + goto error; + } + + fds[0].fd = nbd_aio_get_fd (conn); + fds[0].events = 0; + fds[0].revents = 0; + dir = nbd_aio_get_direction (conn); + if ((dir & LIBNBD_AIO_DIRECTION_READ) != 0) + fds[0].events |= POLLIN; + if ((dir & LIBNBD_AIO_DIRECTION_WRITE) != 0) + fds[0].events |= POLLOUT; + + if (poll (fds, 1, -1)...

...nbd.c @@ -45,6 +45,7 @@ #include <pthread.h> #include <semaphore.h> #include <poll.h> +#include <signal.h> #include <libnbd.h> @@ -67,7 +68,6 @@ struct handle { /* These fields are read-only once initialized */ struct nbd_handle *nbd; int fd; /* Cache of nbd_aio_get_fd */ - int fds[2]; /* Pipe for kicking the reader thread */ bool readonly; pthread_t reader; @@ -105,6 +105,15 @@ static char *tls_psk; static struct handle *nbdplug_open_handle (int readonly); static void nbdplug_close_handle (struct handle *h); +/* Original signal mask, with SIGUSR1 unbl...

...0006f20 T nbd_aio_connect@@LIBNBD_1.0 0000000000007320 T nbd_aio_connect_command@@LIBNBD_1.0 [etc] $ nm -D --with-symbol-versions examples/.libs/glib-main-loop | grep LIBNBD U nbd_aio_connect_command@LIBNBD_1.0 U nbd_aio_get_direction@LIBNBD_1.0 U nbd_aio_get_fd@LIBNBD_1.0 U nbd_aio_in_flight@LIBNBD_1.0 U nbd_aio_is_ready@LIBNBD_1.0 U nbd_aio_notify_read@LIBNBD_1.0 U nbd_aio_notify_write@LIBNBD_1.0 U nbd_aio_peek_command_completed@LIBNBD_1.0 U nbd_aio_prea...