Re: [Qemu-devel] [RFC 08/13] block: drop bdrv_get_aio_context()

[Paolo Bonzini <pbonzini@redhat.com>]

Il 17/06/2013 16:57, Stefan Hajnoczi ha scritto:
> On Fri, Jun 14, 2013 at 10:12:00AM -0400, Paolo Bonzini wrote:
>> Il 14/06/2013 05:48, Stefan Hajnoczi ha scritto:
>>> Associating a BlockDriverState with a single AioContext is not flexible
>>> enough.  Once we make BlockDriverState thread-safe, it will be possible
>>> to call bdrv_*() functions from multiple event loops.
>>
>> I'm afraid that this is trading some pain now (converting
>> qemu_bh_new/qemu_set_fd_handler to aio_bh_new/aio_set_fd_handler) for
>> more pain later (having to make BDS thread-safe).  There aren't that
>> many (~40) in block layer code.
>>
>> Making BlockDriverState thread-safe is hard, it is much simpler to run
>> all the BlockDriverState code in the AioContext thread itself.
>>
>> There are some things that cannot (basically monitor commands and other
>> places that are currently using bdrv_drain_all) but they can simply take
>> a "big AioContext lock".
> 
> I don't like a big AioContext lock that stops the event loop because we
> need to support a M:N device:iothread model where stopping an event loop
> means artificially stopping other devices too.

The event loop would still run via aio_wait(), just in the monitor
thread.  However it would stop the ioeventfd, through a mechanism such
as the one you added in this thread or something like that.

> Maybe it's workable though since the only commands that would take an
> AioContext lock are rare and shouldn't impact performance.

Yes.

> I guess then
> it comes down to robustness if a hung NFS mount can affect the entire VM
> instead of just hanging an emulated disk device.

That would only be a hung NFS mount while running something that drains
a BDS, no?  It would hang the monitor, but it wouldn't be a regression
compared to what we have now obviously.

Paolo