Re: [Qemu-devel] [PATCH v7 11/42] Return path: socket_writev_buffer: Block even on non-blocking fd's

[Dr. David Alan Gilbert]

* Juan Quintela (address@hidden) wrote:
> "Dr. David Alan Gilbert (git)" <address@hidden> wrote:
> > From: "Dr. David Alan Gilbert" <address@hidden>
> >
> > The destination sets the fd to non-blocking on incoming migrations;
> > this also affects the return path from the destination, and thus we
> > need to make sure we can safely write to the return path.
> >
> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> 
> Reviewed-by: Juan Quintela <address@hidden>

Thanks,

> >  migration/qemu-file-unix.c | 41 ++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 36 insertions(+), 5 deletions(-)
> >
> > diff --git a/migration/qemu-file-unix.c b/migration/qemu-file-unix.c
> > index 561621e..b6c55ab 100644
> > --- a/migration/qemu-file-unix.c
> > +++ b/migration/qemu-file-unix.c
> > @@ -39,12 +39,43 @@ static ssize_t socket_writev_buffer(void *opaque, 
> > struct iovec *iov, int iovcnt,
> >      QEMUFileSocket *s = opaque;
> >      ssize_t len;
> >      ssize_t size = iov_size(iov, iovcnt);
> > +    ssize_t offset = 0;
> > +    int     err;
> >  
> > -    len = iov_send(s->fd, iov, iovcnt, 0, size);
> > -    if (len < size) {
> > -        len = -socket_error();
> > -    }
> > -    return len;
> > +    while (size > 0) {
> > +        len = iov_send(s->fd, iov, iovcnt, offset, size);
> > +
> > +        if (len > 0) {
> > +            size -= len;
> > +            offset += len;
> > +        }
> 
> ion_send() can return -1 on error
> 
> This looks a "hacky way" to look for it, althoght my understanding is
> that it is correct, so the review-by

I have to check the socket_error in two cases, one where
len == -1 , and the other where it's only sent some of the data.
Given I have to do it in both cases it doesn't seme worth explicitly
checking for the -1.

Dave

> > +
> > +        if (size > 0) {
> > +            err = socket_error();
> > +
> > +            if (err != EAGAIN && err != EWOULDBLOCK) {
> > +                error_report("socket_writev_buffer: Got err=%d for 
> > (%zd/%zd)",
> > +                             err, size, len);
> > +                /*
> > +                 * If I've already sent some but only just got the error, I
> > +                 * could return the amount validly sent so far and wait 
> > for the
> > +                 * next call to report the error, but I'd rather flag the 
> > error
> > +                 * immediately.
> > +                 */
> > +                return -err;
> > +            }
> > +
> > +            /* Emulate blocking */
> > +            GPollFD pfd;
> > +
> > +            pfd.fd = s->fd;
> > +            pfd.events = G_IO_OUT | G_IO_ERR;
> > +            pfd.revents = 0;
> > +            g_poll(&pfd, 1 /* 1 fd */, -1 /* no timeout */);
> > +        }
> > +     }
> > +
> > +    return offset;
> >  }
> >  
> >  static int socket_get_fd(void *opaque)
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK