Re: [PATCH 3/4] vhost-user: Monitor slave channel in vhost_user_read()

[Greg Kurz <groug@kaod.org>]

[-- Attachment #1: Type: text/plain, Size: 1448 bytes --]

On Tue, 9 Mar 2021 15:18:56 +0000
Stefan Hajnoczi <stefanha@redhat.com> wrote:

> On Mon, Mar 08, 2021 at 01:31:40PM +0100, Greg Kurz wrote:
> > @@ -363,8 +367,30 @@ static int vhost_user_read(struct vhost_dev *dev, VhostUserMsg *msg)
> >      qemu_chr_be_update_read_handlers(chr->chr, ctxt);
> >      qemu_chr_fe_add_watch(chr, G_IO_IN | G_IO_HUP, vhost_user_read_cb, &data);
> >  
> > +    if (u->slave_ioc) {
> > +        /*
> > +         * This guarantees that all pending events in the main context
> > +         * for the slave channel are purged. They will be re-detected
> > +         * and processed now by the nested loop.
> > +         */
> > +        g_source_destroy(u->slave_src);
> > +        g_source_unref(u->slave_src);
> > +        u->slave_src = NULL;
> > +        slave_src = qio_channel_add_watch_source(u->slave_ioc, G_IO_IN,
> 
> Why does slave_ioc use G_IO_IN while chr uses G_IO_IN | G_IO_HUP?

Oops my bad... this is copy&paste of the change introduced in
vhost_setup_slave_channel() by patch 2, which is lacking G_IO_HUP.

It should even actually be G_IO_IN | G_IO_HUP | G_IO_ERR to match
what was done before when calling qemu_set_fd_handler() and which
is recommended by the glib documentation:

https://developer.gnome.org/glib/stable/glib-The-Main-Event-Loop.html#GPollFD

So I'm now wondering why callers of qemu_chr_fe_add_watch() never pass
G_IO_ERR... I'll sort this out for v2.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]