On Wed, Dec 01, 2021 at 09:48:31AM +0000, Rao, Lei wrote:
>
>
> -----Original Message-----
> From: Daniel P. Berrangé <berra...@redhat.com>
> Sent: Wednesday, December 1, 2021 5:11 PM
> To: Rao, Lei <lei....@intel.com>
> Cc: Zhang, Chen <chen.zh...@intel.com>; ebl...@redhat.com;
> vsement...@virtuozzo.com; kw...@redhat.com; hre...@redhat.com;
> qemu-bl...@nongnu.org; qemu-devel@nongnu.org
> Subject: Re: [PATCH] QIO: Add force_quit to the QIOChannel to ensure QIO
> exits cleanly in some corner case
>
> >
> > Signed-off-by: Lei Rao <lei....@intel.com>
> > Signed-off-by: Zhang Chen <chen.zh...@intel.com>
> > ---
> > block/nbd.c | 5 +++++
> > include/io/channel.h | 19 +++++++++++++++++++
> > io/channel.c | 22 ++++++++++++++++++++++
> > 3 files changed, 46 insertions(+)
> >
> > diff --git a/block/nbd.c b/block/nbd.c index 5ef462db1b..5ee4eaaf57
> > 100644
> > --- a/block/nbd.c
> > +++ b/block/nbd.c
> > @@ -208,6 +208,8 @@ static void nbd_teardown_connection(BlockDriverState
> > *bs)
> > assert(!s->in_flight);
> >
> > if (s->ioc) {
> > + qio_channel_set_force_quit(s->ioc, true);
> > + qio_channel_coroutines_wake(s->ioc);
> > qio_channel_shutdown(s->ioc, QIO_CHANNEL_SHUTDOWN_BOTH,
> > NULL);
>
> Calling shutdown here should already be causing the qio_chanel_readv_all to
> wakeup and break out of its
> poll() loop. We shouldn't need to add a second way to break out of the poll().
>
> Calling shutdown can wake up the coroutines which is polling. But I think
> it's not enough. I tried to forcibly wake up the NBD coroutine,
> It may cause segment fault. The root cause is that it will continue to access
> ioc->xxx in qio_channel_yield(), but the ioc has been released and set it
> NULL such as in
> nbd_co_do_establish_connection(); I think call shutdown will have the same
> result. So, I add the force_quit, once set it true, it will immediately exit
> without accessing IOC.
That's a bug in the NBD code then. NBD must not be freeing the IO channel
object while it is performing read/write API calls on it.
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 :|
