Re: [Qemu-devel] [PATCH] blk: fix aio context loss on media change

[Fam Zheng]

On Wed, 03/15 12:03, Kevin Wolf wrote:
> Am 14.03.2017 um 18:11 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > If we have separate iothread for cdrom, we lose connection to it on
> > qmp_blockdev_change_medium, as aio_context is on bds which is dropped
> > and switched with new one.
> > 
> > As an example result, after such media change we have crash on
> > virtio_scsi_ctx_check: Assertion `blk_get_aio_context(d->conf.blk) == 
> > s->ctx' failed.
> > 
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> > ---
> > 
> > Hi all!
> > 
> > We've faced into this assert, and there some kind of fix. I don't sure that
> > such fix doesn't break some conceptions, in this case, I hope, someone will
> > propose a true-way solution.
> 
> The "true way" would be proper AioContext management in the sense that
> all users of a BDS can specify a specific AioContext that they need and
> if they all agree, callbacks are invoked to change everyone to that
> AioContext. If they conflict, attaching the new user would have to error
> out.
> 
> But we discussed this earlier, and while I'm not completely sure any
> more about the details, I seem to remeber that Paolo said something
> along the lines that AioContext is going away anyway and building the
> code for proper management would be wasted time.

Matches my impression.

> 
> Stefan, Paolo, do you remember the details why we didn't even do a
> simple fix like the one below? I think there were some patches on the
> list, no?

ISTM the concern was mostly "what about other BB in the graph?"

Should the new op blocker API be used in this one (a new type of perm)?

> 
> Kevin
> 
> > ======
> > Also, on master branch I can't reproduce it as vm crashed earlier, without 
> > any
> > eject/change, on assert(s->ctx && s->dataplane_started) in
> > virtio_scsi_data_plane_handle_ctrl(). It looks like race with
> > virtio_scsi_dataplane_start(), and for test (to reproduce assert described 
> > above),
> > I've "fixed" it with just:
> > 
> > @@ -63,6 +63,7 @@ static bool 
> > virtio_scsi_data_plane_handle_ctrl(VirtIODevice *vdev,
> >  {
> >      VirtIOSCSI *s = VIRTIO_SCSI(vdev);
> >  
> > +    sleep(10);

This will be fixed by 

https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg02659.html

Fam