Re: [PATCH] chardev/char-socket: Properly make qio connections non blocking

[Daniel P . Berrangé]

On Fri, Apr 17, 2020 at 03:01:09PM +0200, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Apr 17, 2020 at 2:38 PM Sai Pavan Boddu
> <address@hidden> wrote:
> >
> > In tcp_chr_sync_read function, there is a possibility of socket
> > disconnection during read, then tcp_chr_hup function would clean up
> > the qio channel pointers(i.e ioc, sioc).
> >
> > Signed-off-by: Sai Pavan Boddu <address@hidden>
> > ---
> >  chardev/char-socket.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> > index 185fe38..30f2b2b 100644
> > --- a/chardev/char-socket.c
> > +++ b/chardev/char-socket.c
> > @@ -549,11 +549,13 @@ static int tcp_chr_sync_read(Chardev *chr, const 
> > uint8_t *buf, int len)
> >
> >      qio_channel_set_blocking(s->ioc, true, NULL);
> >      size = tcp_chr_recv(chr, (void *) buf, len);
> > -    qio_channel_set_blocking(s->ioc, false, NULL);
> 
> But here it calls tcp_chr_recv(). And I can't find cleanup there.
> Nevertheless, I think this patch should be harmless.
> 
> I'd ask Daniel to have a second look.

I don't see any bug that needs fixing here, and I prefer the current
code as it gives confidence that nothing tcp_chr_disconnect does
will accidentally block.


> >      if (size == 0) {
> >          /* connection closed */
> >          tcp_chr_disconnect(chr);
> > +        return 0;
> >      }
> > +    /* Connection is good */
> > +    qio_channel_set_blocking(s->ioc, false, NULL);
> >
> >      return size;
> >  }
> > --
> > 2.7.4
> >
> >
> 
> 
> -- 
> Marc-André Lureau
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|