Re: [Qemu-devel] [PATCH 4/4] migration: add missed aio_context_acquire into HMP snapshot code

["Denis V. Lunev" <den@openvz.org>]

On 10/28/2015 06:33 PM, Juan Quintela wrote:
> "Denis V. Lunev" <den@openvz.org> wrote:
>> aio_context should be locked in the similar way as was done in QMP
>> snapshot creation in the other case there are a lot of possible
>> troubles if native AIO mode is enabled for disk.
>>
>> - the command can hang (HMP thread) with missed wakeup (the operation is
>>    actually complete)
>>      io_submit
>>      ioq_submit
>>      laio_submit
>>      raw_aio_submit
>>      raw_aio_readv
>>      bdrv_co_io_em
>>      bdrv_co_readv_em
>>      bdrv_aligned_preadv
>>      bdrv_co_do_preadv
>>      bdrv_co_do_readv
>>      bdrv_co_readv
>>      qcow2_co_readv
>>      bdrv_aligned_preadv
>>      bdrv_co_do_pwritev
>>      bdrv_rw_co_entry
>>
>> - QEMU can assert in coroutine re-enter
>>      __GI_abort
>>      qemu_coroutine_enter
>>      bdrv_co_io_em_complete
>>      qemu_laio_process_completion
>>      qemu_laio_completion_bh
>>      aio_bh_poll
>>      aio_dispatch
>>      aio_poll
>>      iothread_run
>>
>> qemu_fopen_bdrv and bdrv_fclose are used in real snapshot operations only
>> along with block drivers. This change should influence only HMP snapshot
>> operations.
>>
>> AioContext lock is reqursive. Thus nested locking should not be a problem.
>>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Stefan Hajnoczi <stefanha@redhat.com>
>> CC: Paolo Bonzini <pbonzini@redhat.com>
>> CC: Juan Quintela <quintela@redhat.com>
>> CC: Amit Shah <amit.shah@redhat.com>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
>
> Should this one go through the block layer?  I guess that the block
> layer, but otherwise, I will get it.

let's wait opinion from Stefan :)

Either way would be good to me, but I want to have
previous patches from the set committed too. They
should be definitely flow through block tree thus
block tree would be better.

Anyway, I can retry the process with patches 1-3
if you'll get 4 through your queue.

Den