From patchwork Tue Sep 15 06:11:42 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Fam Zheng X-Patchwork-Id: 517675 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 60F19140187 for ; Tue, 15 Sep 2015 16:15:00 +1000 (AEST) Received: from localhost ([::1]:45215 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZbjW5-0002XC-VR for incoming@patchwork.ozlabs.org; Tue, 15 Sep 2015 02:14:57 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44649) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZbjTP-0006B0-Pk for qemu-devel@nongnu.org; Tue, 15 Sep 2015 02:12:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZbjTM-00040Y-EY for qemu-devel@nongnu.org; Tue, 15 Sep 2015 02:12:11 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57340) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZbjTM-00040S-4P for qemu-devel@nongnu.org; Tue, 15 Sep 2015 02:12:08 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id B4900461D8; Tue, 15 Sep 2015 06:12:07 +0000 (UTC) Received: from ad.nay.redhat.com. (dhcp-15-42.nay.redhat.com [10.66.15.42]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t8F6BrfU028357; Tue, 15 Sep 2015 02:12:04 -0400 From: Fam Zheng To: qemu-devel@nongnu.org Date: Tue, 15 Sep 2015 14:11:42 +0800 Message-Id: <1442297513-7001-4-git-send-email-famz@redhat.com> In-Reply-To: <1442297513-7001-1-git-send-email-famz@redhat.com> References: <1442297513-7001-1-git-send-email-famz@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Kevin Wolf , Jeff Cody , Max Reitz , vsementsov@parallels.com, stefanha@redhat.com, John Snow Subject: [Qemu-devel] [PATCH v6 03/14] 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. Signed-off-by: John Snow Reviewed-by: Max Reitz Reviewed-by: Stefan Hajnoczi Signed-off-by: Stefan Hajnoczi Reviewed-by: Fam Zheng Signed-off-by: Fam Zheng --- blockdev.c | 116 ++++++++++++++++++++++++++++++++++--------------------------- 1 file changed, 65 insertions(+), 51 deletions(-) diff --git a/blockdev.c b/blockdev.c index b1e7f2a..4ebded8 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1246,43 +1246,57 @@ 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; } InternalSnapshotState; -static void internal_snapshot_prepare(BlkTransactionState *common, +static void internal_snapshot_prepare(BlkActionState *common, Error **errp) { Error *local_err = NULL; @@ -1378,7 +1392,7 @@ static void internal_snapshot_prepare(BlkTransactionState *common, state->bs = bs; } -static void internal_snapshot_abort(BlkTransactionState *common) +static void internal_snapshot_abort(BlkActionState *common) { InternalSnapshotState *state = DO_UPCAST(InternalSnapshotState, common, common); @@ -1401,7 +1415,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); @@ -1413,13 +1427,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) { BlockDriver *drv; @@ -1540,7 +1554,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); @@ -1558,7 +1572,7 @@ static void external_snapshot_commit(BlkTransactionState *common) aio_context_release(state->aio_context); } -static void external_snapshot_abort(BlkTransactionState *common) +static void external_snapshot_abort(BlkActionState *common) { ExternalSnapshotState *state = DO_UPCAST(ExternalSnapshotState, common, common); @@ -1571,13 +1585,13 @@ static void external_snapshot_abort(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); BlockDriverState *bs; @@ -1618,7 +1632,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; @@ -1629,7 +1643,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); @@ -1639,13 +1653,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; @@ -1694,7 +1708,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; @@ -1705,7 +1719,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); @@ -1715,7 +1729,7 @@ static void blockdev_backup_clean(BlkTransactionState *common) } typedef struct BlockDirtyBitmapState { - BlkTransactionState common; + BlkActionState common; BdrvDirtyBitmap *bitmap; BlockDriverState *bs; AioContext *aio_context; @@ -1723,7 +1737,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; @@ -1744,7 +1758,7 @@ 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, @@ -1759,7 +1773,7 @@ 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, @@ -1788,7 +1802,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); @@ -1796,7 +1810,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); @@ -1804,7 +1818,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); @@ -1814,17 +1828,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, @@ -1844,7 +1858,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, }, @@ -1875,10 +1889,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 */ @@ -1887,7 +1901,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;