From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37600) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zz3lX-0002Zr-A2 for qemu-devel@nongnu.org; Wed, 18 Nov 2015 09:31:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zz3lU-0007Gd-2w for qemu-devel@nongnu.org; Wed, 18 Nov 2015 09:31:19 -0500 Received: from mx1.redhat.com ([209.132.183.28]:56346) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zz3lT-0007GX-RH for qemu-devel@nongnu.org; Wed, 18 Nov 2015 09:31:16 -0500 From: Juan Quintela In-Reply-To: <20151118151551.4570fa06@bahia.local> (Greg Kurz's message of "Wed, 18 Nov 2015 15:15:51 +0100") References: <1447751311-2317-1-git-send-email-den@openvz.org> <20151118151551.4570fa06@bahia.local> Date: Wed, 18 Nov 2015 15:31:01 +0100 Message-ID: <87h9kj5qhm.fsf@emacs.mitica> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH for 2.9 v8 0/10] dataplane snapshot fixes Reply-To: quintela@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Greg Kurz Cc: Kevin Wolf , "Denis V. Lunev" , qemu-devel@nongnu.org, stefanha@redhat.com Greg Kurz wrote: > On Tue, 17 Nov 2015 12:08:20 +0300 > "Denis V. Lunev" wrote: > >> with test >> while /bin/true ; do >> virsh snapshot-create rhel7 >> sleep 10 >> virsh snapshot-delete rhel7 --current >> done >> with enabled iothreads on a running VM leads to a lot of troubles: hangs, >> asserts, errors. >> > > In my case, when using a virtio-blk-dataplane device, calling savevm *always* > result in a QEMU hang. Oops > With this series (plus the s/bs/bs_vm_state/ change in patch 11), savevm/loadvm > now works like a charm. Nice, thanks for the testing. > I saw that Juan does not like aio_context being used in migration code, but > in case this series gets applied anyway: > > Tested-by: Greg Kurz I *think* that we should get better API's exported from block layer, but *at least* we will get this series in. Thanks, Juan. > >> Anyway, I think that the construction like >> assert(aio_context_is_locked(aio_context)); >> should be widely used to ensure proper locking. >> >> Changes from v8: >> - split patch 5 into 2 separate patches making changes better to understand and >> review >> >> Changes from v7: >> - patch 5: fixes for bdrv_all_find_snapshot. Changed parameter name to >> skip_read_only which better reflects the meaning of the value and >> fixed checks inside to conform the note from Stefan >> >> Changes from v6: >> - tricky part dropped from patch 7 >> - patch 5 reworked to process snapshot list differently in info commands >> and on savevm >> >> Changes from v5: >> - dropped already merged patch 11 >> - fixed spelling in patch 1 >> - changed order of condition in loops in all patches. Thank you Stefan >> - dropped patch 9 >> - aio_context is not acquired any more in bdrv_all_find_vmstate_bs by request >> of Stefan >> - patch 10 is implemented in completely different way >> >> Changes from v4: >> - only migration/savevm.c code and monitor is affected now. Generic block >> layer stuff will be sent separately to speedup merging. The approach >> in general was negotiated with Juan and Stefan. >> >> Changes from v3: >> - more places found >> - new aio_poll concept, see patch 10 >> >> Changes from v2: >> - droppped patch 5 as already merged >> - changed locking scheme in patch 4 by suggestion of Juan >> >> Changes from v1: >> - aio-context locking added >> - comment is rewritten >> >> Signed-off-by: Denis V. Lunev >> CC: Stefan Hajnoczi >> CC: Juan Quintela >> CC: Kevin Wolf >> >> Denis V. Lunev (10): >> snapshot: create helper to test that block drivers supports snapshots >> snapshot: return error code from bdrv_snapshot_delete_by_id_or_name >> snapshot: create bdrv_all_delete_snapshot helper >> snapshot: create bdrv_all_goto_snapshot helper >> snapshot: create bdrv_all_find_snapshot helper >> migration: drop find_vmstate_bs check in hmp_delvm >> snapshot: create bdrv_all_create_snapshot helper >> migration: reorder processing in hmp_savevm >> migration: implement bdrv_all_find_vmstate_bs helper >> migration: normalize locking in migration/savevm.c >> >> block/snapshot.c | 135 ++++++++++++++++++++++++++++++- >> include/block/snapshot.h | 25 +++++- >> migration/savevm.c | 207 +++++++++++++++-------------------------------- >> 3 files changed, 217 insertions(+), 150 deletions(-)