From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60675) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xlbtm-0005AL-QE for qemu-devel@nongnu.org; Tue, 04 Nov 2014 06:03:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xlbtg-0001ud-KX for qemu-devel@nongnu.org; Tue, 04 Nov 2014 06:03:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:46065) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xlbtg-0001uP-E7 for qemu-devel@nongnu.org; Tue, 04 Nov 2014 06:03:36 -0500 Message-ID: <5458B279.8010300@redhat.com> Date: Tue, 04 Nov 2014 12:03:21 +0100 From: Max Reitz MIME-Version: 1.0 References: <1414639364-4499-1-git-send-email-famz@redhat.com> <1414639364-4499-9-git-send-email-famz@redhat.com> In-Reply-To: <1414639364-4499-9-git-send-email-famz@redhat.com> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v6 08/10] qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable} List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: Kevin Wolf , Benoit Canet , Vladimir Sementsov-Ogievskij , Markus Armbruster , Luiz Capitulino , John Snow , Stefan Hajnoczi , Jd , Paolo Bonzini On 2014-10-30 at 04:22, Fam Zheng wrote: > This adds three qmp commands to transactions. > > Users can stop a dirty bitmap, start backup of it, and start another > dirty bitmap atomically, so that the dirty bitmap is tracked > incrementally and we don't miss any write. > > Signed-off-by: Fam Zheng > --- > blockdev.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > qapi-schema.json | 5 ++- > 2 files changed, 96 insertions(+), 1 deletion(-) > > diff --git a/blockdev.c b/blockdev.c > index fca909b..34fa314 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -1490,6 +1490,83 @@ static void drive_backup_abort(BlkTransactionState *common) > } > } > > +static void block_dirty_bitmap_add_prepare(BlkTransactionState *common, > + Error **errp) > +{ > + BlockDirtyBitmapAdd *action; > + > + action = common->action->block_dirty_bitmap_add; > + qmp_block_dirty_bitmap_add(action->device, action->name, > + action->has_granularity, action->granularity, > + errp); > +} > + > +static void block_dirty_bitmap_add_abort(BlkTransactionState *common) > +{ > + BlockDirtyBitmapAdd *action; > + BdrvDirtyBitmap *bm; > + BlockDriverState *bs; > + > + action = common->action->block_dirty_bitmap_add; > + bs = bdrv_find(action->device); > + if (bs) { > + bm = bdrv_find_dirty_bitmap(bs, action->name); > + if (bm) { > + bdrv_release_dirty_bitmap(bs, bm); > + } > + } > +} > + > +static void block_dirty_bitmap_enable_prepare(BlkTransactionState *common, > + Error **errp) > +{ > + BlockDirtyBitmap *action; > + > + action = common->action->block_dirty_bitmap_enable; > + qmp_block_dirty_bitmap_enable(action->device, action->name, errp); > +} > + > +static void block_dirty_bitmap_enable_abort(BlkTransactionState *common) > +{ > + BlockDirtyBitmap *action; > + BdrvDirtyBitmap *bitmap; > + BlockDriverState *bs; > + > + action = common->action->block_dirty_bitmap_enable; > + bs = bdrv_find(action->device); > + if (bs) { > + bitmap = bdrv_find_dirty_bitmap(bs, action->name); > + if (bitmap) { > + bdrv_disable_dirty_bitmap(bs, bitmap); But what if the dirty bitmap was enabled before so that this enabling transaction was supposed to be a no-op? > + } > + } > +} > + > +static void block_dirty_bitmap_disable_prepare(BlkTransactionState *common, > + Error **errp) > +{ > + BlockDirtyBitmap *action; > + > + action = common->action->block_dirty_bitmap_disable; > + qmp_block_dirty_bitmap_disable(action->device, action->name, errp); > +} > + > +static void block_dirty_bitmap_disable_abort(BlkTransactionState *common) > +{ > + BlockDirtyBitmap *action; > + BdrvDirtyBitmap *bitmap; > + BlockDriverState *bs; > + > + action = common->action->block_dirty_bitmap_disable; > + bs = bdrv_find(action->device); > + if (bs) { > + bitmap = bdrv_find_dirty_bitmap(bs, action->name); > + if (bitmap) { > + bdrv_enable_dirty_bitmap(bs, bitmap); Same here, the other way round. > + } > + } > +} > + > static void abort_prepare(BlkTransactionState *common, Error **errp) > { > error_setg(errp, "Transaction aborted using Abort action"); > @@ -1522,6 +1599,21 @@ static const BdrvActionOps actions[] = { > .prepare = internal_snapshot_prepare, > .abort = internal_snapshot_abort, > }, > + [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_ADD] = { > + .instance_size = sizeof(BlkTransactionState), > + .prepare = block_dirty_bitmap_add_prepare, > + .abort = block_dirty_bitmap_add_abort, > + }, > + [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_ENABLE] = { > + .instance_size = sizeof(BlkTransactionState), > + .prepare = block_dirty_bitmap_enable_prepare, > + .abort = block_dirty_bitmap_enable_abort, > + }, > + [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_DISABLE] = { > + .instance_size = sizeof(BlkTransactionState), > + .prepare = block_dirty_bitmap_disable_prepare, > + .abort = block_dirty_bitmap_disable_abort, > + }, > }; > > /* > diff --git a/qapi-schema.json b/qapi-schema.json > index 24379ab..ae07677 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -1260,7 +1260,10 @@ > 'blockdev-snapshot-sync': 'BlockdevSnapshot', > 'drive-backup': 'DriveBackup', > 'abort': 'Abort', > - 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal' > + 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal', > + 'block-dirty-bitmap-add': 'BlockDirtyBitmapAdd', Ah, you're reusing that type here. Very well then. Max > + 'block-dirty-bitmap-enable': 'BlockDirtyBitmap', > + 'block-dirty-bitmap-disable': 'BlockDirtyBitmap' > } } > > ##