Re: [PATCH 2/2] io: follow coroutine AioContext in qio_channel_yield()

[Stefan Hajnoczi <stefanha@redhat.com>]

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

On Thu, Aug 24, 2023 at 01:09:59PM -0300, Fabiano Rosas wrote:
> Stefan Hajnoczi <stefanha@redhat.com> writes:
> 
> > @@ -2089,10 +2088,6 @@ static void nbd_attach_aio_context(BlockDriverState *bs,
> >       * the reconnect_delay_timer cannot be active here.
> >       */
> >      assert(!s->reconnect_delay_timer);
> > -
> > -    if (s->ioc) {
> > -        qio_channel_attach_aio_context(s->ioc, new_context);
> > -    }
> >  }
> >  
> >  static void nbd_detach_aio_context(BlockDriverState *bs)
> > @@ -2101,10 +2096,6 @@ static void nbd_detach_aio_context(BlockDriverState *bs)
> >  
> >      assert(!s->open_timer);
> >      assert(!s->reconnect_delay_timer);
> > -
> > -    if (s->ioc) {
> > -        qio_channel_detach_aio_context(s->ioc);
> > -    }
> >  }
> 
> The whole attach/detach functions could go away.

Yes, but at the expense of losing the assertions. Some of them have
extensive comments and I guess they are important to someone, so I
didn't drop them.

> 
> >  
> >  static BlockDriver bdrv_nbd = {
> > diff --git a/io/channel-command.c b/io/channel-command.c
> > index 7ed726c802..1f61026222 100644
> > --- a/io/channel-command.c
> > +++ b/io/channel-command.c
> > @@ -331,14 +331,21 @@ static int qio_channel_command_close(QIOChannel *ioc,
> >  
> >  
> >  static void qio_channel_command_set_aio_fd_handler(QIOChannel *ioc,
> > -                                                   AioContext *ctx,
> > +                                                   AioContext *read_ctx,
> >                                                     IOHandler *io_read,
> > +                                                   AioContext *write_ctx,
> >                                                     IOHandler *io_write,
> >                                                     void *opaque)
> >  {
> >      QIOChannelCommand *cioc = QIO_CHANNEL_COMMAND(ioc);
> > -    aio_set_fd_handler(ctx, cioc->readfd, io_read, NULL, NULL, NULL, opaque);
> > -    aio_set_fd_handler(ctx, cioc->writefd, NULL, io_write, NULL, NULL, opaque);
> > +    if (read_ctx) {
> > +        aio_set_fd_handler(read_ctx, cioc->readfd, io_read, NULL,
> > +                NULL, NULL, opaque);
> > +    }
> > +    if (write_ctx) {
> > +        aio_set_fd_handler(write_ctx, cioc->writefd, NULL, io_write,
> > +                NULL, NULL, opaque);
> > +    }
> >  }
> >  
> >  
> 
> ...
> 
> > diff --git a/nbd/client.c b/nbd/client.c
> > index 479208d5d9..81877d088d 100644
> > --- a/nbd/client.c
> > +++ b/nbd/client.c
> > @@ -948,7 +948,7 @@ static int nbd_start_negotiate(AioContext *aio_context, QIOChannel *ioc,
> >                  ioc = *outioc;
> >                  if (aio_context) {
> >                      qio_channel_set_blocking(ioc, false, NULL);
> > -                    qio_channel_attach_aio_context(ioc, aio_context);
> > +                    qio_channel_set_follow_coroutine_ctx(ioc, true);
> 
> This is actually unreachable, aio_context is always NULL here.

Interesting, I will add a patch to remove the dead code. Thanks!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]