Re: [Qemu-devel] [PATCH v3 15/15] mirror: follow AioContext change gracefully

[Stefan Hajnoczi]

On Tue, Jun 14, 2016 at 1:09 PM, Paolo Bonzini <address@hidden> wrote:
> On 13/06/2016 19:05, Stefan Hajnoczi wrote:
>> index 80fd3c7..046e95c 100644
>> --- a/block/mirror.c
>> +++ b/block/mirror.c
>> @@ -331,6 +331,8 @@ static uint64_t coroutine_fn 
>> mirror_iteration(MirrorBlockJob *s)
>>          mirror_wait_for_io(s);
>>      }
>>
>> +    block_job_pause_point(&s->common);
>
> I/O can be in-flight here, so your description of the function is not
> 100% accurate.  I suppose it's okay because s->waiting_for_io is false
> at the pause points you specified?

You are right, this brings up a subtle issue that already exists
without my patches:

The co-routine may sleep with aio requests pending.  This makes the
job "paused" (!job->busy) while there is still activity!

The solution for AioContext switching that this patch relies on is:

+static void mirror_detach_aio_context(BlockJob *job)
+{
+    MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
+
+    while (s->in_flight > 0) {
+        aio_poll(blk_get_aio_context(job->blk), true);
+    }
+}

blockjob.c:block_job_detach_aio_context() first calls
block_job_pause(), then mirror_detach_aio_context() above, and finally
waits for !job->busy.

I chose this approach because it safely handles existing sleep or
block_job_yield() callers while there is still activity.

Now that I think more about it, we should solve the problem not just
for AioContext switching but also for pausing sleeping or
block_job_yield() coroutines.

I'll try to come up with a solution in v4.

Stefan