From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:55480)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <den@openvz.org>) id 1ZrT6Q-0007MT-9c
	for qemu-devel@nongnu.org; Wed, 28 Oct 2015 11:57:36 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <den@openvz.org>) id 1ZrT6M-0003Ek-67
	for qemu-devel@nongnu.org; Wed, 28 Oct 2015 11:57:30 -0400
References: <1446044465-19312-1-git-send-email-den@openvz.org>
	<1446044465-19312-5-git-send-email-den@openvz.org>
	<871tcf80t5.fsf@neno.neno>
From: "Denis V. Lunev" <den@openvz.org>
Message-ID: <5630F04D.5060309@openvz.org>
Date: Wed, 28 Oct 2015 18:57:01 +0300
MIME-Version: 1.0
In-Reply-To: <871tcf80t5.fsf@neno.neno>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 4/4] migration: add missed
 aio_context_acquire into HMP snapshot code
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: quintela@redhat.com
Cc: Amit Shah <amit.shah@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>, qemu-stable@nongnu.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