Libguestfs - Jun 2019 - Re: [libnbd PATCH] docs: Improve nbd_aio_get_direction documentation

Mention things to remember for avoiding the deadlock of polling for a
POLLIN from a server that has no replies to send.

Perhaps we could split the READY state into two - one for when there
are no commands in flight (and get_direction returns 0 to state that
polling is pointless, although a multi-threaded reader can still poll
for POLLIN), and the other when there ARE commands in flight.  Such a
separation would also make it easier to know when nbd_aio_disconnect
is not going to strand an in-flight command's response. But I'm not
sure it is worth the effort.
---

I'm pushing this one now, but sending the email because of the
question it raises on whether we want the user to be able to
distinguish a difference between any vs. no in-flight commands.

 generator/generator | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/generator/generator b/generator/generator
index a289741..1d4db23 100755
--- a/generator/generator
+++ b/generator/generator
@@ -1719,6 +1719,18 @@ We are expected next to read from the server.  If using
L<poll(2)>
 you would set C<events = POLLIN>.  If C<revents> returns
C<POLLIN>
 you would then call C<nbd_aio_notify_read>.

+Note that once libnbd reaches C<nbd_aio_is_ready>, this direction is
+returned even before a command is issued via C<nbd_aio_pwrite> and
+friends. In a single-threaded use of libnbd, it is not worth polling
+until after issuing a command, as otherwise the server will never wake
+up the poll. In a multi-threaded scenario, you can have one thread
+begin a polling loop prior to any commands, but any other thread that
+issues a command will need a way to kick the polling thread out of
+poll in case issuing the command changes the needed polling
+direction. Possible ways to do this include polling for activity on a
+pipe-to-self, or using L<pthread_kill(3)> to send a signal that is
+masked except during L<ppoll(2)>.
+
 =item C<LIBNBD_AIO_DIRECTION_WRITE> = 2

 We are expected next to write to the server.  If using L<poll(2)>
@@ -1727,13 +1739,12 @@ you would then call C<nbd_aio_notify_write>.

 =item C<LIBNBD_AIO_DIRECTION_BOTH> = 3

-We are expected next to either read or write to the server.
-If using L<poll(2)> you would set C<events = POLLIN|POLLOUT>.
-If one of C<POLLIN> or C<POLLOUT> is returned, then see above.
-However note that you shouldn't call C<nbd_aio_notify_read>
-and C<nbd_aio_notify_write> because calling the first one will
-change the state of the connection, possibly invalidating the second
-notification.
+We are expected next to either read or write to the server.  If using
+L<poll(2)> you would set C<events = POLLIN|POLLOUT>.  If only one
of
+C<POLLIN> or C<POLLOUT> is returned, then see above.  However, if
both
+are returned, it is better to call only C<nbd_aio_notify_read>, as
+processing the server's reply may change the state of the connection
+and invalidate the need to write more commands.

 =back";
   };
-- 
2.20.1

On 6/20/19 6:50 AM, Eric Blake wrote:
> Mention things to remember for avoiding the deadlock of polling for a
> POLLIN from a server that has no replies to send.
> 
> Perhaps we could split the READY state into two - one for when there
> are no commands in flight (and get_direction returns 0 to state that
> polling is pointless, although a multi-threaded reader can still poll
> for POLLIN), and the other when there ARE commands in flight.  Such a
> separation would also make it easier to know when nbd_aio_disconnect
> is not going to strand an in-flight command's response. But I'm not
> sure it is worth the effort.
> ---
> 
> I'm pushing this one now, but sending the email because of the
> question it raises on whether we want the user to be able to
> distinguish a difference between any vs. no in-flight commands.
Actually, I'm not pushing it yet. I noticed that we can also return 0
(such as on a just-created handle), and need to document that as well.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

On Thu, Jun 20, 2019 at 04:33:11PM -0500, Eric Blake
wrote:
> On 6/20/19 6:50 AM, Eric Blake wrote:
> > Mention things to remember for avoiding the deadlock of polling for a
> > POLLIN from a server that has no replies to send.
> > 
> > Perhaps we could split the READY state into two - one for when there
> > are no commands in flight (and get_direction returns 0 to state that
> > polling is pointless, although a multi-threaded reader can still poll
> > for POLLIN), and the other when there ARE commands in flight.  Such a
> > separation would also make it easier to know when nbd_aio_disconnect
> > is not going to strand an in-flight command's response. But
I'm not
> > sure it is worth the effort.
> > ---
> > 
> > I'm pushing this one now, but sending the email because of the
> > question it raises on whether we want the user to be able to
> > distinguish a difference between any vs. no in-flight commands.
> 
> Actually, I'm not pushing it yet. I noticed that we can also return 0
> (such as on a just-created handle), and need to document that as well.
Patch is still good, with or without the extra documentation, so ACK
anyway.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html