On 16/03/2016 17:39, Stefan Hajnoczi wrote: > The tree looks like this: > > [NBD export] > / > v > [guest] temporary qcow2 > \ / > v v > disk > > Block backend access is in square brackets. Nodes without square > brackets are BDS nodes. > > If the guest wants to drain the disk, it's possible for new I/O requests > to enter the disk BDS while we're recursing to disk's children because > the NBD export socket fd is in the same AIOContext. The socket fd is > therefore handled during aio_poll() calls. > > I'm not 100% sure that this is a problem, but I wonder if you've thought > about this?
I hadn't, but I think this is handled by using bdrv_drained_begin/bdrv_drained_end instead of bdrv_drain. The NBD export registers its callback as "external", and it is thus disabled between bdrv_drained_begin and bdrv_drained_end. It will indeed become more complex when BDSes won't have anymore a "home AioContext" due to multiqueue. I suspect that we should rethink the strategy for enabling and disabling external callbacks. For example we could add callbacks to each BlockBackend that enable/disable external callbacks, and when bdrv_drained_begin is called on a BDS, we call the callbacks for all BlockBackends that are included in this BDS. I'm not sure if there's a way to go from a BDS to all the BBs above it. Paolo