From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57000) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zz0QF-00062K-0h for qemu-devel@nongnu.org; Wed, 18 Nov 2015 05:57:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zz0QA-0001BR-Tn for qemu-devel@nongnu.org; Wed, 18 Nov 2015 05:57:06 -0500 Received: from mx1.redhat.com ([209.132.183.28]:54654) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zz0QA-0001BL-P8 for qemu-devel@nongnu.org; Wed, 18 Nov 2015 05:57:02 -0500 From: Juan Quintela In-Reply-To: <1447751311-2317-1-git-send-email-den@openvz.org> (Denis V. Lunev's message of "Tue, 17 Nov 2015 12:08:20 +0300") References: <1447751311-2317-1-git-send-email-den@openvz.org> Date: Wed, 18 Nov 2015 11:56:48 +0100 Message-ID: <87bnarpocv.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: "Denis V. Lunev" Cc: Kevin Wolf , qemu-devel@nongnu.org, stefanha@redhat.com "Denis V. Lunev" wrote: for 2.9? You really plane very well in advance? O:-) As you asked for review from migration side .... > 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. > > 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(-)