[Libguestfs] [nbdkit PATCH 1/2] nbd: Don't reference stale errno in reader loop

[Richard W.M. Jones]

On Fri, Mar 27, 2020 at 05:33:27PM -0500, Eric Blake wrote:
> When switching to libnbd in commit ab7760fcfd, I mistakenly assumed
> that after a POLLIN event fires on the pipe-to-self, then read() will
> return either 1 or -1.  But this is not true; read() can also return 0
> (if the pipe hits EOF), in which case POSIX says errno has an
> unspecified value, and we should not be deciding whether to log an
> error based on a random value.  I did not manage to fix the bug even
> when later refactoring the pipe-to-self to be non-blocking, although
> it appears to be latent as long as the pipe is non-blocking and the
> write end is not closed.
> 
> Fixes: e0d32468
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  plugins/nbd/nbd.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c
> index d020beec..e5b8f338 100644
> --- a/plugins/nbd/nbd.c
> +++ b/plugins/nbd/nbd.c
> @@ -332,9 +332,11 @@ nbdplug_reader (void *handle)
> 
>      /* Check if we were kicked because a command was started */
>      if (fds[1].revents & POLLIN) {
> -      while (read (h->fds[0], &c, 1) == 1)
> +      int r;
> +
> +      while ((r = read (h->fds[0], &c, 1)) == 1)
>          /* Drain any backlog */;
> -      if (errno != EAGAIN) {
> +      if (r == -1 && errno != EAGAIN) {
>          nbdkit_error ("failed to read pipe: %m");
>          break;
>        }

This one seems pretty obvious, so ACK.

Although personally I would have made one other change: The semicolon
after the comment was invisible to me until I had studied the code
above and the original code for a while.  I actually wrote a quite
lengthy email here, which was completely wrong once I noticed the
semicolon.  Can we make that more visible somehow?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v