From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40297) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZYqMe-0004mk-Vc for qemu-devel@nongnu.org; Mon, 07 Sep 2015 02:57:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZYqMZ-00020M-VN for qemu-devel@nongnu.org; Mon, 07 Sep 2015 02:57:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42330) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZYqMZ-0001yy-O7 for qemu-devel@nongnu.org; Mon, 07 Sep 2015 02:57:11 -0400 Date: Mon, 7 Sep 2015 14:57:07 +0800 From: Fam Zheng Message-ID: <20150907065707.GG3958@ad.nay.redhat.com> References: <1438238370-16212-1-git-send-email-famz@redhat.com> <1438238370-16212-12-git-send-email-famz@redhat.com> <55BFB255.1090202@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55BFB255.1090202@redhat.com> Subject: Re: [Qemu-devel] [PATCH v4 11/14] block/backup: support block job transactions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: Kevin Wolf , Jeff Cody , qemu-devel@nongnu.org, vsementsov@parallels.com, stefanha@redhat.com, John Snow On Mon, 08/03 20:26, Max Reitz wrote: > On 30.07.2015 08:39, Fam Zheng wrote: > >From: Stefan Hajnoczi > > > >Join the transaction when the 'transactional-cancel' QMP argument is > >true. > > > >This ensures that the sync bitmap is not thrown away if another block > >job in the transaction is cancelled or fails. This is critical so > >incremental backup with multiple disks can be retried in case of > >cancellation/failure. > > > >Signed-off-by: Stefan Hajnoczi > >Signed-off-by: Fam Zheng > >--- > > block/backup.c | 25 ++++++-- > > blockdev.c | 141 +++++++++++++++++++++++++++++++++++++--------- > > hmp.c | 2 +- > > include/block/block_int.h | 3 +- > > qapi/block-core.json | 14 ++++- > > 5 files changed, 148 insertions(+), 37 deletions(-) > > [...] > > >diff --git a/blockdev.c b/blockdev.c > >index aba9d78..3e8b8b6 100644 > >--- a/blockdev.c > >+++ b/blockdev.c > > [...] > > >@@ -2770,6 +2830,31 @@ out: > > aio_context_release(aio_context); > > } > > > >+void qmp_blockdev_backup(const char *device, const char *target, > >+ enum MirrorSyncMode sync, > >+ bool has_speed, int64_t speed, > >+ bool has_on_source_error, > >+ BlockdevOnError on_source_error, > >+ bool has_on_target_error, > >+ BlockdevOnError on_target_error, > >+ bool has_transactional_cancel, > >+ bool transactional_cancel, > >+ Error **errp) > >+{ > >+ if (has_transactional_cancel && transactional_cancel) { > >+ error_setg(errp, "Transactional cancel can only be used in the " > >+ "'transaction' command"); > >+ return; > >+ } > >+ > >+ do_blockdev_backup(device, target, sync, has_speed, speed, > >+ has_on_source_error, on_source_error, > >+ has_on_target_error, on_target_error, > >+ NULL, errp); > >+} > >+ > >+#define DEFAULT_MIRROR_BUF_SIZE (10 << 20) > > Why? Superfluous, removing. > > >+ > > void qmp_drive_mirror(const char *device, const char *target, > > bool has_format, const char *format, > > bool has_node_name, const char *node_name, > > [...] > > >diff --git a/qapi/block-core.json b/qapi/block-core.json > >index 7b2efb8..d4674ed 100644 > >--- a/qapi/block-core.json > >+++ b/qapi/block-core.json > >@@ -736,6 +736,10 @@ > > # default 'report' (no limitations, since this applies to > > # a different block device than @device). > > # > >+# @transactional-cancel: #optional whether failure or cancellation of other > >+# block jobs with @transactional-cancel true causes the > >+# whole group to cancel. (Since 2.5) > >+# > > Maybe it's worth adding "[of other block jobs] in the same transaction", and > maybe even noting that this therefore only works with backup block jobs > started as part of a transaction (although that should be clear then). OK, adding "in the same transaction". Fam