From patchwork Tue Nov 10 14:14:07 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Hajnoczi X-Patchwork-Id: 542374 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 67A7D141418 for ; Wed, 11 Nov 2015 01:33:12 +1100 (AEDT) Received: from localhost ([::1]:60440 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zw9yw-0007fH-3f for incoming@patchwork.ozlabs.org; Tue, 10 Nov 2015 09:33:10 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58149) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zw9hb-0001XM-Qs for qemu-devel@nongnu.org; Tue, 10 Nov 2015 09:15:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zw9hW-0005d4-HM for qemu-devel@nongnu.org; Tue, 10 Nov 2015 09:15:15 -0500 Received: from mx1.redhat.com ([209.132.183.28]:60134) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zw9hW-0005cs-54 for qemu-devel@nongnu.org; Tue, 10 Nov 2015 09:15:10 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id B6D9AC150C0C; Tue, 10 Nov 2015 14:15:09 +0000 (UTC) Received: from localhost (ovpn-112-78.ams2.redhat.com [10.36.112.78]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id tAAEF8kE010449; Tue, 10 Nov 2015 09:15:08 -0500 From: Stefan Hajnoczi To: Date: Tue, 10 Nov 2015 14:14:07 +0000 Message-Id: <1447164879-6756-13-git-send-email-stefanha@redhat.com> In-Reply-To: <1447164879-6756-1-git-send-email-stefanha@redhat.com> References: <1447164879-6756-1-git-send-email-stefanha@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Fam Zheng , Peter Maydell , John Snow , Stefan Hajnoczi Subject: [Qemu-devel] [PULL 12/44] block: rename BlkTransactionState and BdrvActionOps X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org From: John Snow These structures are misnomers, somewhat. (1) BlockTransactionState is not state for a transaction, but is rather state for a single transaction action. Rename it "BlkActionState" to be more accurate. (2) The BdrvActionOps describes operations for the BlkActionState, above. This name might imply a 'BdrvAction' or a 'BdrvActionState', which there isn't. Rename this to 'BlkActionOps' to match 'BlkActionState'. Lastly, update the surrounding in-line documentation and comments to reflect the current nature of how Transactions operate. This patch changes only comments and names, and should not affect behavior in any way. Reviewed-by: Max Reitz Reviewed-by: Stefan Hajnoczi Signed-off-by: Stefan Hajnoczi Reviewed-by: Fam Zheng Signed-off-by: Fam Zheng Signed-off-by: John Snow Message-id: 1446765200-3054-4-git-send-email-jsnow@redhat.com Signed-off-by: Stefan Hajnoczi --- blockdev.c | 124 ++++++++++++++++++++++++++++++++++--------------------------- 1 file changed, 69 insertions(+), 55 deletions(-) diff --git a/blockdev.c b/blockdev.c index f4dff8b..ec4a79c 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1347,44 +1347,58 @@ static BdrvDirtyBitmap *block_dirty_bitmap_lookup(const char *node, /* New and old BlockDriverState structs for atomic group operations */ -typedef struct BlkTransactionState BlkTransactionState; +typedef struct BlkActionState BlkActionState; -/* Only prepare() may fail. In a single transaction, only one of commit() or - abort() will be called, clean() will always be called if it present. */ -typedef struct BdrvActionOps { - /* Size of state struct, in bytes. */ +/** + * BlkActionOps: + * Table of operations that define an Action. + * + * @instance_size: Size of state struct, in bytes. + * @prepare: Prepare the work, must NOT be NULL. + * @commit: Commit the changes, can be NULL. + * @abort: Abort the changes on fail, can be NULL. + * @clean: Clean up resources after all transaction actions have called + * commit() or abort(). Can be NULL. + * + * Only prepare() may fail. In a single transaction, only one of commit() or + * abort() will be called. clean() will always be called if it is present. + */ +typedef struct BlkActionOps { size_t instance_size; - /* Prepare the work, must NOT be NULL. */ - void (*prepare)(BlkTransactionState *common, Error **errp); - /* Commit the changes, can be NULL. */ - void (*commit)(BlkTransactionState *common); - /* Abort the changes on fail, can be NULL. */ - void (*abort)(BlkTransactionState *common); - /* Clean up resource in the end, can be NULL. */ - void (*clean)(BlkTransactionState *common); -} BdrvActionOps; + void (*prepare)(BlkActionState *common, Error **errp); + void (*commit)(BlkActionState *common); + void (*abort)(BlkActionState *common); + void (*clean)(BlkActionState *common); +} BlkActionOps; -/* - * This structure must be arranged as first member in child type, assuming - * that compiler will also arrange it to the same address with parent instance. - * Later it will be used in free(). +/** + * BlkActionState: + * Describes one Action's state within a Transaction. + * + * @action: QAPI-defined enum identifying which Action to perform. + * @ops: Table of ActionOps this Action can perform. + * @entry: List membership for all Actions in this Transaction. + * + * This structure must be arranged as first member in a subclassed type, + * assuming that the compiler will also arrange it to the same offsets as the + * base class. */ -struct BlkTransactionState { +struct BlkActionState { TransactionAction *action; - const BdrvActionOps *ops; - QSIMPLEQ_ENTRY(BlkTransactionState) entry; + const BlkActionOps *ops; + QSIMPLEQ_ENTRY(BlkActionState) entry; }; /* internal snapshot private data */ typedef struct InternalSnapshotState { - BlkTransactionState common; + BlkActionState common; BlockDriverState *bs; AioContext *aio_context; QEMUSnapshotInfo sn; bool created; } InternalSnapshotState; -static void internal_snapshot_prepare(BlkTransactionState *common, +static void internal_snapshot_prepare(BlkActionState *common, Error **errp) { Error *local_err = NULL; @@ -1483,7 +1497,7 @@ static void internal_snapshot_prepare(BlkTransactionState *common, state->created = true; } -static void internal_snapshot_abort(BlkTransactionState *common) +static void internal_snapshot_abort(BlkActionState *common) { InternalSnapshotState *state = DO_UPCAST(InternalSnapshotState, common, common); @@ -1506,7 +1520,7 @@ static void internal_snapshot_abort(BlkTransactionState *common) } } -static void internal_snapshot_clean(BlkTransactionState *common) +static void internal_snapshot_clean(BlkActionState *common) { InternalSnapshotState *state = DO_UPCAST(InternalSnapshotState, common, common); @@ -1521,13 +1535,13 @@ static void internal_snapshot_clean(BlkTransactionState *common) /* external snapshot private data */ typedef struct ExternalSnapshotState { - BlkTransactionState common; + BlkActionState common; BlockDriverState *old_bs; BlockDriverState *new_bs; AioContext *aio_context; } ExternalSnapshotState; -static void external_snapshot_prepare(BlkTransactionState *common, +static void external_snapshot_prepare(BlkActionState *common, Error **errp) { int flags, ret; @@ -1643,7 +1657,7 @@ static void external_snapshot_prepare(BlkTransactionState *common, } } -static void external_snapshot_commit(BlkTransactionState *common) +static void external_snapshot_commit(BlkActionState *common) { ExternalSnapshotState *state = DO_UPCAST(ExternalSnapshotState, common, common); @@ -1659,7 +1673,7 @@ static void external_snapshot_commit(BlkTransactionState *common) NULL); } -static void external_snapshot_abort(BlkTransactionState *common) +static void external_snapshot_abort(BlkActionState *common) { ExternalSnapshotState *state = DO_UPCAST(ExternalSnapshotState, common, common); @@ -1668,7 +1682,7 @@ static void external_snapshot_abort(BlkTransactionState *common) } } -static void external_snapshot_clean(BlkTransactionState *common) +static void external_snapshot_clean(BlkActionState *common) { ExternalSnapshotState *state = DO_UPCAST(ExternalSnapshotState, common, common); @@ -1679,13 +1693,13 @@ static void external_snapshot_clean(BlkTransactionState *common) } typedef struct DriveBackupState { - BlkTransactionState common; + BlkActionState common; BlockDriverState *bs; AioContext *aio_context; BlockJob *job; } DriveBackupState; -static void drive_backup_prepare(BlkTransactionState *common, Error **errp) +static void drive_backup_prepare(BlkActionState *common, Error **errp) { DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common); BlockBackend *blk; @@ -1730,7 +1744,7 @@ static void drive_backup_prepare(BlkTransactionState *common, Error **errp) state->job = state->bs->job; } -static void drive_backup_abort(BlkTransactionState *common) +static void drive_backup_abort(BlkActionState *common) { DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common); BlockDriverState *bs = state->bs; @@ -1741,7 +1755,7 @@ static void drive_backup_abort(BlkTransactionState *common) } } -static void drive_backup_clean(BlkTransactionState *common) +static void drive_backup_clean(BlkActionState *common) { DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common); @@ -1752,13 +1766,13 @@ static void drive_backup_clean(BlkTransactionState *common) } typedef struct BlockdevBackupState { - BlkTransactionState common; + BlkActionState common; BlockDriverState *bs; BlockJob *job; AioContext *aio_context; } BlockdevBackupState; -static void blockdev_backup_prepare(BlkTransactionState *common, Error **errp) +static void blockdev_backup_prepare(BlkActionState *common, Error **errp) { BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common); BlockdevBackup *backup; @@ -1810,7 +1824,7 @@ static void blockdev_backup_prepare(BlkTransactionState *common, Error **errp) state->job = state->bs->job; } -static void blockdev_backup_abort(BlkTransactionState *common) +static void blockdev_backup_abort(BlkActionState *common) { BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common); BlockDriverState *bs = state->bs; @@ -1821,7 +1835,7 @@ static void blockdev_backup_abort(BlkTransactionState *common) } } -static void blockdev_backup_clean(BlkTransactionState *common) +static void blockdev_backup_clean(BlkActionState *common) { BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common); @@ -1832,7 +1846,7 @@ static void blockdev_backup_clean(BlkTransactionState *common) } typedef struct BlockDirtyBitmapState { - BlkTransactionState common; + BlkActionState common; BdrvDirtyBitmap *bitmap; BlockDriverState *bs; AioContext *aio_context; @@ -1840,7 +1854,7 @@ typedef struct BlockDirtyBitmapState { bool prepared; } BlockDirtyBitmapState; -static void block_dirty_bitmap_add_prepare(BlkTransactionState *common, +static void block_dirty_bitmap_add_prepare(BlkActionState *common, Error **errp) { Error *local_err = NULL; @@ -1848,7 +1862,7 @@ static void block_dirty_bitmap_add_prepare(BlkTransactionState *common, BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState, common, common); - action = common->action->block_dirty_bitmap_add; + action = common->action->u.block_dirty_bitmap_add; /* AIO context taken and released within qmp_block_dirty_bitmap_add */ qmp_block_dirty_bitmap_add(action->node, action->name, action->has_granularity, action->granularity, @@ -1861,13 +1875,13 @@ static void block_dirty_bitmap_add_prepare(BlkTransactionState *common, } } -static void block_dirty_bitmap_add_abort(BlkTransactionState *common) +static void block_dirty_bitmap_add_abort(BlkActionState *common) { BlockDirtyBitmapAdd *action; BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState, common, common); - action = common->action->block_dirty_bitmap_add; + action = common->action->u.block_dirty_bitmap_add; /* Should not be able to fail: IF the bitmap was added via .prepare(), * then the node reference and bitmap name must have been valid. */ @@ -1876,14 +1890,14 @@ static void block_dirty_bitmap_add_abort(BlkTransactionState *common) } } -static void block_dirty_bitmap_clear_prepare(BlkTransactionState *common, +static void block_dirty_bitmap_clear_prepare(BlkActionState *common, Error **errp) { BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState, common, common); BlockDirtyBitmap *action; - action = common->action->block_dirty_bitmap_clear; + action = common->action->u.block_dirty_bitmap_clear; state->bitmap = block_dirty_bitmap_lookup(action->node, action->name, &state->bs, @@ -1905,7 +1919,7 @@ static void block_dirty_bitmap_clear_prepare(BlkTransactionState *common, /* AioContext is released in .clean() */ } -static void block_dirty_bitmap_clear_abort(BlkTransactionState *common) +static void block_dirty_bitmap_clear_abort(BlkActionState *common) { BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState, common, common); @@ -1913,7 +1927,7 @@ static void block_dirty_bitmap_clear_abort(BlkTransactionState *common) bdrv_undo_clear_dirty_bitmap(state->bitmap, state->backup); } -static void block_dirty_bitmap_clear_commit(BlkTransactionState *common) +static void block_dirty_bitmap_clear_commit(BlkActionState *common) { BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState, common, common); @@ -1921,7 +1935,7 @@ static void block_dirty_bitmap_clear_commit(BlkTransactionState *common) hbitmap_free(state->backup); } -static void block_dirty_bitmap_clear_clean(BlkTransactionState *common) +static void block_dirty_bitmap_clear_clean(BlkActionState *common) { BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState, common, common); @@ -1931,17 +1945,17 @@ static void block_dirty_bitmap_clear_clean(BlkTransactionState *common) } } -static void abort_prepare(BlkTransactionState *common, Error **errp) +static void abort_prepare(BlkActionState *common, Error **errp) { error_setg(errp, "Transaction aborted using Abort action"); } -static void abort_commit(BlkTransactionState *common) +static void abort_commit(BlkActionState *common) { g_assert_not_reached(); /* this action never succeeds */ } -static const BdrvActionOps actions[] = { +static const BlkActionOps actions[] = { [TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC] = { .instance_size = sizeof(ExternalSnapshotState), .prepare = external_snapshot_prepare, @@ -1962,7 +1976,7 @@ static const BdrvActionOps actions[] = { .clean = blockdev_backup_clean, }, [TRANSACTION_ACTION_KIND_ABORT] = { - .instance_size = sizeof(BlkTransactionState), + .instance_size = sizeof(BlkActionState), .prepare = abort_prepare, .commit = abort_commit, }, @@ -1993,10 +2007,10 @@ static const BdrvActionOps actions[] = { void qmp_transaction(TransactionActionList *dev_list, Error **errp) { TransactionActionList *dev_entry = dev_list; - BlkTransactionState *state, *next; + BlkActionState *state, *next; Error *local_err = NULL; - QSIMPLEQ_HEAD(snap_bdrv_states, BlkTransactionState) snap_bdrv_states; + QSIMPLEQ_HEAD(snap_bdrv_states, BlkActionState) snap_bdrv_states; QSIMPLEQ_INIT(&snap_bdrv_states); /* drain all i/o before any operations */ @@ -2005,7 +2019,7 @@ void qmp_transaction(TransactionActionList *dev_list, Error **errp) /* We don't do anything in this loop that commits us to the operations */ while (NULL != dev_entry) { TransactionAction *dev_info = NULL; - const BdrvActionOps *ops; + const BlkActionOps *ops; dev_info = dev_entry->value; dev_entry = dev_entry->next;