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

[Denis V. Lunev]

On 10/28/2015 06:33 PM, Juan Quintela wrote:
> "Denis V. Lunev" <address@hidden> 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 <address@hidden>
> > CC: Stefan Hajnoczi <address@hidden>
> > CC: Paolo Bonzini <address@hidden>
> > CC: Juan Quintela <address@hidden>
> > CC: Amit Shah <address@hidden>
> Reviewed-by: Juan Quintela <address@hidden>
>
> 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