Re: [Qemu-devel] [PATCH 07/11] block: let backup blockjob run in BDS AioContext

[Max Reitz <mreitz@redhat.com>]

On 02.10.2014 17:08, Stefan Hajnoczi wrote:
> On Wed, Oct 01, 2014 at 09:38:39PM +0200, Max Reitz wrote:
>> On 01.10.2014 19:01, Stefan Hajnoczi wrote:
>>> The backup block job must run in the BlockDriverState AioContext so that
>>> it works with dataplane.
>>>
>>> The basics of acquiring the AioContext are easy in blockdev.c.
>>>
>>> The completion code in block/backup.c must call bdrv_unref() from the
>>> main loop.  Use block_job_defer_to_main_loop() to achieve that.
>>>
>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>>> ---
>>>   block/backup.c | 21 +++++++++++++++++++--
>>>   blockdev.c     | 23 ++++++++++++++++-------
>>>   2 files changed, 35 insertions(+), 9 deletions(-)
>> Hm, so, if I run a backup blockjob from one thread (which is not the main
>> loop and not the BDS's AIO context)
> That cannot happen.  The blockjob always runs with the BDS AioContext
> acquired - either because we explicitly did so in the main loop or
> because the dataplane IOThread acquires it.
>
>> and try to cancel it or set its speed
>> from another thread (e.g. the main loop), won't the blockjob hold the BDS's
>> AIO context lock as long as it runs and making it impossible to interfere?
> No, because of how the AioContext's RfifoLock lock works.
>
> It has a "contention callback" that kicks the aio_poll() loop.  This
> allows the QEMU monitor to acquire the lock whenever an IOThread is
> blocked in poll(2).  It uses an eventfd to force the IOThread out of
> poll(2) and this happens automatically in aio_context_acquire().
>
> See aio_context_new() and aio_rfifolock_cb().

Ah, okay, thank you for clarifying that.

Reviewed-by: Max Reitz <mreitz@redhat.com>