* [Qemu-devel] [PATCH v2 00/11] block: incremental backup transactions
@ 2015-03-27 19:19 John Snow
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 01/11] qapi: Add transaction support to block-dirty-bitmap operations John Snow
` (11 more replies)
0 siblings, 12 replies; 31+ messages in thread
From: John Snow @ 2015-03-27 19:19 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, mreitz, vsementsov, stefanha
requires: 1426879023-18151-1-git-send-email-jsnow@redhat.com
"[PATCH v4 00/20] block: transactionless incremental backup series"
This series adds support for incremental backup primitives in QMP
transactions. It requires my transactionless incremental backup series,
currently at v4.
Patch 1 adds basic support for add and clear transactions.
Patch 2 tests this basic support.
Patches 3-4 refactor transactions a little bit, to add clarity.
Patch 5 adds the framework for error scenarios where only
some jobs that were launched by a transaction complete successfully,
and we need to perform context-sensitive cleanup after the transaction
itself has already completed.
Patches 6-7 add necessary bookkeeping information to backup job
data structures to take advantage of this new feature.
Patch 8 just moves code.
Patch 9 modifies qmp_drive_backup to support the new feature.
Patch 10 implements the new feature for drive_backup transaction actions.
Patch 11 tests the new feature.
Lingering questions:
- Is it worth it to add a post-transaction completion event to QMP that
signifies all jobs launched by the transaction have completed? This
would be of primary interest to libvirt, in particular, but only as
a convenience feature.
Thank you,
--John Snow
v2:
Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
001/11:[0009] [FC] 'qapi: Add transaction support to block-dirty-bitmap operations'
002/11:[0036] [FC] 'iotests: add transactional incremental backup test'
003/11:[down] 'blockdev: rename BlkTransactionState and BdrvActionOps'
004/11:[down] 'block: re-add BlkTransactionState'
005/11:[0274] [FC] 'block: add transactional callbacks feature'
006/11:[0004] [FC] 'block: add refcount to Job object'
007/11:[0048] [FC] 'block: add delayed bitmap successor cleanup'
008/11:[down] 'block: move transactions beneath qmp interfaces'
009/11:[0084] [FC] 'qmp: Add an implementation wrapper for qmp_drive_backup'
010/11:[0050] [FC] 'block: drive_backup transaction callback support'
011/11:[0004] [FC] 'iotests: 124 - transactional failure test'
01: Fixed indentation.
Fixed QMP commands to behave with new bitmap_lookup from
transactionless-v4.
2.3 --> 2.4.
02: Folded in improvements to qemu-iotest 124 from transactional-v1.
03: NEW
04: NEW
05: A lot:
Don't delete the comma in the transaction actions config
use g_new0 instead of g_malloc0
Phrasing: "retcode" --> "Return code"
Use GCC attributes to mark functions as unused until future patches.
Added some data structure documentation.
Many structure and function renames, hopefully to improve readability.
Use just one list for all Actions instead of two separate lists.
Remove ActionState from the list upon deletion/decref
And many other small tweaks.
06: Comment phrasing.
07: Removed errp parameter from all functions introduced by this commit.
bdrv_dirty_bitmap_decref --> bdrv_frozen_bitmap_decref
08: NEW
09: _drive_backup --> do_drive_backup()
Forward declarations removed.
10: Rebased on top of drastically modified #05.
Phrasing: "BackupBlockJob" instead of "BackupJob" in comments.
11: Removed extra parameters to wait_incremental() in
test_transaction_failure()
John Snow (11):
qapi: Add transaction support to block-dirty-bitmap operations
iotests: add transactional incremental backup test
block: rename BlkTransactionState and BdrvActionOps
block: re-add BlkTransactionState
block: add transactional callbacks feature
block: add refcount to Job object
block: add delayed bitmap successor cleanup
block: move transactions beneath qmp interfaces
qmp: Add an implementation wrapper for qmp_drive_backup
block: drive_backup transaction callback support
iotests: 124 - transactional failure test
block.c | 65 +-
block/backup.c | 29 +-
blockdev.c | 1599 ++++++++++++++++++++++++++++----------------
blockjob.c | 18 +-
include/block/block.h | 10 +-
include/block/block_int.h | 8 +
include/block/blockjob.h | 21 +
qapi-schema.json | 6 +-
tests/qemu-iotests/124 | 170 +++++
tests/qemu-iotests/124.out | 4 +-
10 files changed, 1313 insertions(+), 617 deletions(-)
--
2.1.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH v2 01/11] qapi: Add transaction support to block-dirty-bitmap operations
2015-03-27 19:19 [Qemu-devel] [PATCH v2 00/11] block: incremental backup transactions John Snow
@ 2015-03-27 19:19 ` John Snow
2015-04-17 14:41 ` Max Reitz
2015-04-17 14:50 ` Eric Blake
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 02/11] iotests: add transactional incremental backup test John Snow
` (10 subsequent siblings)
11 siblings, 2 replies; 31+ messages in thread
From: John Snow @ 2015-03-27 19:19 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, mreitz, vsementsov, stefanha
This adds two qmp commands to transactions.
block-dirty-bitmap-add allows you to create a bitmap simultaneously
alongside a new full backup to accomplish a clean synchronization
point.
block-dirty-bitmap-clear allows you to reset a bitmap back to as-if
it were new, which can also be used alongside a full backup to
accomplish a clean synchronization point.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
blockdev.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
qapi-schema.json | 6 +++-
2 files changed, 105 insertions(+), 1 deletion(-)
diff --git a/blockdev.c b/blockdev.c
index ab67b4d..d5ea75e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1692,6 +1692,95 @@ static void blockdev_backup_clean(BlkTransactionState *common)
}
}
+typedef struct BlockDirtyBitmapState {
+ BlkTransactionState common;
+ BdrvDirtyBitmap *bitmap;
+ BlockDriverState *bs;
+ AioContext *aio_context;
+ bool prepared;
+} BlockDirtyBitmapState;
+
+static void block_dirty_bitmap_add_prepare(BlkTransactionState *common,
+ Error **errp)
+{
+ Error *local_err = NULL;
+ BlockDirtyBitmapAdd *action;
+ BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+
+ action = common->action->block_dirty_bitmap_add;
+ /* AIO context taken within qmp_block_dirty_bitmap_add */
+ qmp_block_dirty_bitmap_add(action->node, action->name,
+ action->has_granularity, action->granularity,
+ &local_err);
+
+ if (!local_err) {
+ state->prepared = true;
+ } else {
+ error_propagate(errp, local_err);
+ }
+}
+
+static void block_dirty_bitmap_add_abort(BlkTransactionState *common)
+{
+ BlockDirtyBitmapAdd *action;
+ BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+
+ action = common->action->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.
+ */
+ if (state->prepared) {
+ qmp_block_dirty_bitmap_remove(action->node, action->name, &error_abort);
+ }
+}
+
+static void block_dirty_bitmap_clear_prepare(BlkTransactionState *common,
+ Error **errp)
+{
+ BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+ BlockDirtyBitmap *action;
+
+ action = common->action->block_dirty_bitmap_clear;
+ state->bitmap = block_dirty_bitmap_lookup(action->node,
+ action->name,
+ &state->bs,
+ &state->aio_context,
+ errp);
+ if (!state->bitmap) {
+ return;
+ }
+
+ if (bdrv_dirty_bitmap_frozen(state->bitmap)) {
+ error_setg(errp, "Cannot modify a frozen bitmap");
+ return;
+ } else if (!bdrv_dirty_bitmap_enabled(state->bitmap)) {
+ error_setg(errp, "Cannot clear a disabled bitmap");
+ return;
+ }
+
+ /* AioContext is released in .clean() */
+}
+
+static void block_dirty_bitmap_clear_commit(BlkTransactionState *common)
+{
+ BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+ bdrv_clear_dirty_bitmap(state->bitmap);
+}
+
+static void block_dirty_bitmap_clear_clean(BlkTransactionState *common)
+{
+ BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+
+ if (state->aio_context) {
+ aio_context_release(state->aio_context);
+ }
+}
+
static void abort_prepare(BlkTransactionState *common, Error **errp)
{
error_setg(errp, "Transaction aborted using Abort action");
@@ -1732,6 +1821,17 @@ static const BdrvActionOps actions[] = {
.abort = internal_snapshot_abort,
.clean = internal_snapshot_clean,
},
+ [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_ADD] = {
+ .instance_size = sizeof(BlockDirtyBitmapState),
+ .prepare = block_dirty_bitmap_add_prepare,
+ .abort = block_dirty_bitmap_add_abort,
+ },
+ [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_CLEAR] = {
+ .instance_size = sizeof(BlockDirtyBitmapState),
+ .prepare = block_dirty_bitmap_clear_prepare,
+ .commit = block_dirty_bitmap_clear_commit,
+ .clean = block_dirty_bitmap_clear_clean,
+ }
};
/*
diff --git a/qapi-schema.json b/qapi-schema.json
index ac9594d..f6fe2b3 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1356,6 +1356,8 @@
# abort since 1.6
# blockdev-snapshot-internal-sync since 1.7
# blockdev-backup since 2.3
+# block-dirty-bitmap-add since 2.4
+# block-dirty-bitmap-clear since 2.4
##
{ 'union': 'TransactionAction',
'data': {
@@ -1363,7 +1365,9 @@
'drive-backup': 'DriveBackup',
'blockdev-backup': 'BlockdevBackup',
'abort': 'Abort',
- 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal'
+ 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
+ 'block-dirty-bitmap-add': 'BlockDirtyBitmapAdd',
+ 'block-dirty-bitmap-clear': 'BlockDirtyBitmap'
} }
##
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH v2 02/11] iotests: add transactional incremental backup test
2015-03-27 19:19 [Qemu-devel] [PATCH v2 00/11] block: incremental backup transactions John Snow
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 01/11] qapi: Add transaction support to block-dirty-bitmap operations John Snow
@ 2015-03-27 19:19 ` John Snow
2015-04-17 14:42 ` Max Reitz
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 03/11] block: rename BlkTransactionState and BdrvActionOps John Snow
` (9 subsequent siblings)
11 siblings, 1 reply; 31+ messages in thread
From: John Snow @ 2015-03-27 19:19 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, mreitz, vsementsov, stefanha
Test simple usage cases for using transactions to create
and synchronize incremental backups.
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/124 | 51 ++++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/124.out | 4 ++--
2 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index d16d2c2..31946f9 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -226,6 +226,57 @@ class TestIncrementalBackup(iotests.QMPTestCase):
return True
+ def test_incremental_transaction(self):
+ '''Test: Verify backups made from transactionally created bitmaps.
+
+ Create a bitmap "before" VM execution begins, then create a second
+ bitmap AFTER writes have already occurred. Use transactions to create
+ a full backup and synchronize both bitmaps to this backup.
+ Create an incremental backup through both bitmaps and verify that
+ both backups match the current drive0 image.
+ '''
+
+ drive0 = self.drives[0]
+ bitmap0 = self.add_bitmap('bitmap0', self.drives[0])
+ self.hmp_io_writes(drive0['id'], (('0xab', 0, 512),
+ ('0xfe', '16M', '256k'),
+ ('0x64', '32736k', '64k')))
+ bitmap1 = self.add_bitmap('bitmap1', drive0)
+
+ result = self.vm.qmp('transaction', actions=[
+ {
+ 'type': 'block-dirty-bitmap-clear',
+ 'data': { 'node': bitmap0.drive['id'],
+ 'name': bitmap0.name },
+ },
+ {
+ 'type': 'block-dirty-bitmap-clear',
+ 'data': { 'node': bitmap1.drive['id'],
+ 'name': bitmap1.name },
+ },
+ {
+ 'type': 'drive-backup',
+ 'data': { 'device': drive0['id'],
+ 'sync': 'full',
+ 'format': drive0['fmt'],
+ 'target': drive0['backup'] },
+ }
+ ])
+ self.assert_qmp(result, 'return', {})
+ self.wait_until_completed()
+ self.files.append(drive0['backup'])
+ self.check_full_backup()
+
+ self.hmp_io_writes(drive0['id'], (('0x9a', 0, 512),
+ ('0x55', '8M', '352k'),
+ ('0x78', '15872k', '1M')))
+ # Both bitmaps should be in sync and create fully valid
+ # incremental backups
+ res1 = self.create_incremental(bitmap0)
+ res2 = self.create_incremental(bitmap1)
+ self.assertTrue(res1 and res2)
+
+
def test_incremental_failure(self):
'''Test: Verify backups made after a failure are correct.
diff --git a/tests/qemu-iotests/124.out b/tests/qemu-iotests/124.out
index 89968f3..914e373 100644
--- a/tests/qemu-iotests/124.out
+++ b/tests/qemu-iotests/124.out
@@ -1,5 +1,5 @@
-....
+.....
----------------------------------------------------------------------
-Ran 4 tests
+Ran 5 tests
OK
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH v2 03/11] block: rename BlkTransactionState and BdrvActionOps
2015-03-27 19:19 [Qemu-devel] [PATCH v2 00/11] block: incremental backup transactions John Snow
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 01/11] qapi: Add transaction support to block-dirty-bitmap operations John Snow
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 02/11] iotests: add transactional incremental backup test John Snow
@ 2015-03-27 19:19 ` John Snow
2015-04-17 14:51 ` Max Reitz
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 04/11] block: re-add BlkTransactionState John Snow
` (8 subsequent siblings)
11 siblings, 1 reply; 31+ messages in thread
From: John Snow @ 2015-03-27 19:19 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, mreitz, vsementsov, stefanha
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 <jsnow@redhat.com>
---
blockdev.c | 114 ++++++++++++++++++++++++++++++++++---------------------------
1 file changed, 64 insertions(+), 50 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index d5ea75e..6247ca7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1227,43 +1227,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;
@@ -1357,7 +1371,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);
@@ -1380,7 +1394,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);
@@ -1392,13 +1406,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;
@@ -1519,7 +1533,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);
@@ -1537,7 +1551,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);
@@ -1550,13 +1564,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;
@@ -1596,7 +1610,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;
@@ -1607,7 +1621,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);
@@ -1617,13 +1631,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;
@@ -1672,7 +1686,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;
@@ -1683,7 +1697,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);
@@ -1693,14 +1707,14 @@ static void blockdev_backup_clean(BlkTransactionState *common)
}
typedef struct BlockDirtyBitmapState {
- BlkTransactionState common;
+ BlkActionState common;
BdrvDirtyBitmap *bitmap;
BlockDriverState *bs;
AioContext *aio_context;
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;
@@ -1721,7 +1735,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,
@@ -1736,7 +1750,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,
@@ -1764,14 +1778,14 @@ static void block_dirty_bitmap_clear_prepare(BlkTransactionState *common,
/* AioContext is released in .clean() */
}
-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);
bdrv_clear_dirty_bitmap(state->bitmap);
}
-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);
@@ -1781,17 +1795,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,
@@ -1811,7 +1825,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,
},
@@ -1841,10 +1855,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 */
@@ -1853,7 +1867,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;
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH v2 04/11] block: re-add BlkTransactionState
2015-03-27 19:19 [Qemu-devel] [PATCH v2 00/11] block: incremental backup transactions John Snow
` (2 preceding siblings ...)
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 03/11] block: rename BlkTransactionState and BdrvActionOps John Snow
@ 2015-03-27 19:19 ` John Snow
2015-04-17 15:11 ` Max Reitz
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 05/11] block: add transactional callbacks feature John Snow
` (7 subsequent siblings)
11 siblings, 1 reply; 31+ messages in thread
From: John Snow @ 2015-03-27 19:19 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, mreitz, vsementsov, stefanha
Now that the structure formerly known as BlkTransactionState has been
renamed to something sensible (BlkActionState), re-introduce an actual
BlkTransactionState that actually manages state for the entire Transaction.
In the process, convert the old QSIMPLEQ list of actions into a QTAILQ,
to let us more efficiently delete items in arbitrary order, which will
be more important in the future when some actions will expire at the end
of the transaction, but others may persist until all callbacks triggered
by the transaction are recollected.
Signed-off-by: John Snow <jsnow@redhat.com>
---
blockdev.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 59 insertions(+), 7 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 6247ca7..f806d40 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1252,6 +1252,19 @@ typedef struct BlkActionOps {
} BlkActionOps;
/**
+ * BlkTransactionState:
+ * Object to track the job completion info for jobs launched
+ * by a transaction group.
+ *
+ * @jobs: A reference count that tracks how many jobs still need to complete.
+ * @actions: A list of all Actions in the Transaction.
+ */
+typedef struct BlkTransactionState {
+ int jobs;
+ QTAILQ_HEAD(actions, BlkActionState) actions;
+} BlkTransactionState;
+
+/**
* BlkActionState:
* Describes one Action's state within a Transaction.
*
@@ -1266,9 +1279,45 @@ typedef struct BlkActionOps {
struct BlkActionState {
TransactionAction *action;
const BlkActionOps *ops;
- QSIMPLEQ_ENTRY(BlkActionState) entry;
+ QTAILQ_ENTRY(BlkActionState) entry;
};
+static BlkTransactionState *new_blk_transaction_state(void)
+{
+ BlkTransactionState *bts = g_new0(BlkTransactionState, 1);
+
+ /* The qmp_transaction function itself can be considered a pending job
+ * that should complete before pending action callbacks are executed,
+ * so increment the jobs remaining refcount to indicate this. */
+ bts->jobs = 1;
+ QTAILQ_INIT(&bts->actions);
+ return bts;
+}
+
+static void destroy_blk_transaction_state(BlkTransactionState *bts)
+{
+ BlkActionState *bas, *bas_next;
+
+ /* The list should in normal cases be empty,
+ * but in case someone really just wants to kibosh the whole deal: */
+ QTAILQ_FOREACH_SAFE(bas, &bts->actions, entry, bas_next) {
+ QTAILQ_REMOVE(&bts->actions, bas, entry);
+ g_free(bas);
+ }
+
+ g_free(bts);
+}
+
+static BlkTransactionState *transaction_job_complete(BlkTransactionState *bts)
+{
+ bts->jobs--;
+ if (bts->jobs == 0) {
+ destroy_blk_transaction_state(bts);
+ return NULL;
+ }
+ return bts;
+}
+
/* internal snapshot private data */
typedef struct InternalSnapshotState {
BlkActionState common;
@@ -1856,10 +1905,10 @@ void qmp_transaction(TransactionActionList *dev_list, Error **errp)
{
TransactionActionList *dev_entry = dev_list;
BlkActionState *state, *next;
+ BlkTransactionState *bts;
Error *local_err = NULL;
- QSIMPLEQ_HEAD(snap_bdrv_states, BlkActionState) snap_bdrv_states;
- QSIMPLEQ_INIT(&snap_bdrv_states);
+ bts = new_blk_transaction_state();
/* drain all i/o before any operations */
bdrv_drain_all();
@@ -1880,7 +1929,7 @@ void qmp_transaction(TransactionActionList *dev_list, Error **errp)
state = g_malloc0(ops->instance_size);
state->ops = ops;
state->action = dev_info;
- QSIMPLEQ_INSERT_TAIL(&snap_bdrv_states, state, entry);
+ QTAILQ_INSERT_TAIL(&bts->actions, state, entry);
state->ops->prepare(state, &local_err);
if (local_err) {
@@ -1889,7 +1938,7 @@ void qmp_transaction(TransactionActionList *dev_list, Error **errp)
}
}
- QSIMPLEQ_FOREACH(state, &snap_bdrv_states, entry) {
+ QTAILQ_FOREACH(state, &bts->actions, entry) {
if (state->ops->commit) {
state->ops->commit(state);
}
@@ -1900,18 +1949,21 @@ void qmp_transaction(TransactionActionList *dev_list, Error **errp)
delete_and_fail:
/* failure, and it is all-or-none; roll back all operations */
- QSIMPLEQ_FOREACH(state, &snap_bdrv_states, entry) {
+ QTAILQ_FOREACH(state, &bts->actions, entry) {
if (state->ops->abort) {
state->ops->abort(state);
}
}
exit:
- QSIMPLEQ_FOREACH_SAFE(state, &snap_bdrv_states, entry, next) {
+ QTAILQ_FOREACH_SAFE(state, &bts->actions, entry, next) {
if (state->ops->clean) {
state->ops->clean(state);
}
+ QTAILQ_REMOVE(&bts->actions, state, entry);
g_free(state);
}
+
+ transaction_job_complete(bts);
}
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH v2 05/11] block: add transactional callbacks feature
2015-03-27 19:19 [Qemu-devel] [PATCH v2 00/11] block: incremental backup transactions John Snow
` (3 preceding siblings ...)
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 04/11] block: re-add BlkTransactionState John Snow
@ 2015-03-27 19:19 ` John Snow
2015-04-17 15:41 ` Max Reitz
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 06/11] block: add refcount to Job object John Snow
` (6 subsequent siblings)
11 siblings, 1 reply; 31+ messages in thread
From: John Snow @ 2015-03-27 19:19 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, mreitz, vsementsov, stefanha
The goal here is to add a new method to transactions that allows
developers to specify a callback that will get invoked only once
all jobs spawned by a transaction are completed, allowing developers
the chance to perform actions conditionally pending complete success,
partial failure, or complete failure.
In order to register the new callback to be invoked, a user must request
a callback pointer and closure by calling new_action_cb_wrapper, which
creates a wrapper around an opaque pointer and callback that would have
originally been passed to e.g. backup_start().
The function will return a function pointer and a new opaque pointer to
be passed instead. The transaction system will effectively intercept the
original callbacks and perform book-keeping on the transaction after it
has delivered the original enveloped callback.
This means that Transaction Action callback methods will be called after
all callbacks triggered by all Actions in the Transactional group have
been received.
This feature has no knowledge of any jobs spawned by Actions that do not
inform the system via new_action_cb_wrapper().
For an example of how to use the feature, please skip ahead to:
'block: drive_backup transaction callback support' which serves as an example
for how to hook up a post-transaction callback to the Drive Backup action.
Note 1: Defining a callback method alone is not sufficient to have the new
method invoked. You must call new_action_cb_wrapper() AND ensure the
callback it returns is the one used as the callback for the job
launched by the action.
Note 2: You can use this feature for any system that registers completions of
an asynchronous task via a callback of the form
(void *opaque, int ret), not just block job callbacks.
Signed-off-by: John Snow <jsnow@redhat.com>
---
blockdev.c | 191 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 187 insertions(+), 4 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index f806d40..d404251 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1239,6 +1239,8 @@ typedef struct BlkActionState BlkActionState;
* @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.
+ * @cb: Executed after all jobs launched by actions in the transaction finish,
+ * but only if requested by new_action_cb_wrapper() prior to clean().
*
* 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.
@@ -1249,6 +1251,7 @@ typedef struct BlkActionOps {
void (*commit)(BlkActionState *common);
void (*abort)(BlkActionState *common);
void (*clean)(BlkActionState *common);
+ void (*cb)(BlkActionState *common);
} BlkActionOps;
/**
@@ -1257,19 +1260,46 @@ typedef struct BlkActionOps {
* by a transaction group.
*
* @jobs: A reference count that tracks how many jobs still need to complete.
+ * @status: A cumulative return code for all actions that have reported
+ * a return code via callback in the transaction.
* @actions: A list of all Actions in the Transaction.
+ * However, once the transaction has completed, it will be only a list
+ * of transactions that have registered a post-transaction callback.
*/
typedef struct BlkTransactionState {
int jobs;
+ int status;
QTAILQ_HEAD(actions, BlkActionState) actions;
} BlkTransactionState;
+typedef void (CallbackFn)(void *opaque, int ret);
+
+/**
+ * BlkActionCallbackData:
+ * Necessary state for intercepting and
+ * re-delivering a callback triggered by an Action.
+ *
+ * @opaque: The data to be given to the encapsulated callback when
+ * a job launched by an Action completes.
+ * @ret: The status code that was delivered to the encapsulated callback.
+ * @callback: The encapsulated callback to invoke upon completion of
+ * the Job launched by the Action.
+ */
+typedef struct BlkActionCallbackData {
+ void *opaque;
+ int ret;
+ CallbackFn *callback;
+} BlkActionCallbackData;
+
/**
* 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.
+ * @transaction: A pointer back to the Transaction this Action belongs to.
+ * @cb_data: Information on this Action's encapsulated callback, if any.
+ * @refcount: reference count, allowing access to this state beyond clean().
* @entry: List membership for all Actions in this Transaction.
*
* This structure must be arranged as first member in a subclassed type,
@@ -1279,6 +1309,9 @@ typedef struct BlkTransactionState {
struct BlkActionState {
TransactionAction *action;
const BlkActionOps *ops;
+ BlkTransactionState *transaction;
+ BlkActionCallbackData *cb_data;
+ int refcount;
QTAILQ_ENTRY(BlkActionState) entry;
};
@@ -1294,6 +1327,26 @@ static BlkTransactionState *new_blk_transaction_state(void)
return bts;
}
+static BlkActionState *put_blk_action_state(BlkActionState *state)
+{
+ BlkActionState *bas = state;
+
+ state->refcount--;
+ if (state->refcount == 0) {
+
+ QTAILQ_FOREACH(bas, &state->transaction->actions, entry) {
+ if (bas == state) {
+ QTAILQ_REMOVE(&state->transaction->actions, bas, entry);
+ break;
+ }
+ }
+ g_free(state->cb_data);
+ g_free(state);
+ return NULL;
+ }
+ return state;
+}
+
static void destroy_blk_transaction_state(BlkTransactionState *bts)
{
BlkActionState *bas, *bas_next;
@@ -1301,23 +1354,151 @@ static void destroy_blk_transaction_state(BlkTransactionState *bts)
/* The list should in normal cases be empty,
* but in case someone really just wants to kibosh the whole deal: */
QTAILQ_FOREACH_SAFE(bas, &bts->actions, entry, bas_next) {
- QTAILQ_REMOVE(&bts->actions, bas, entry);
- g_free(bas);
+ put_blk_action_state(bas);
}
g_free(bts);
}
+static void transaction_run_cb_action_ops(BlkTransactionState *bts)
+{
+ BlkActionState *bas, *bas_next;
+
+ QTAILQ_FOREACH_SAFE(bas, &bts->actions, entry, bas_next) {
+ if (bas->ops->cb) {
+ bas->ops->cb(bas);
+ }
+
+ g_free(bas->cb_data);
+ bas->cb_data = NULL;
+ put_blk_action_state(bas);
+ }
+}
+
static BlkTransactionState *transaction_job_complete(BlkTransactionState *bts)
{
bts->jobs--;
if (bts->jobs == 0) {
+ transaction_run_cb_action_ops(bts);
destroy_blk_transaction_state(bts);
return NULL;
}
return bts;
}
+static void transaction_job_cancel(BlkActionState *bas)
+{
+ assert(bas->transaction->jobs > 1);
+ transaction_job_complete(bas->transaction);
+}
+
+/**
+ * Intercept a callback that was issued due to a transactional action.
+ */
+static void transaction_action_callback(void *opaque, int ret)
+{
+ BlkActionState *common = opaque;
+ BlkActionCallbackData *cb_data = common->cb_data;
+ CallbackFn *callback = cb_data->callback;
+
+ /* Prepare data for ops->cb() */
+ cb_data->callback = NULL;
+ cb_data->ret = ret;
+
+ /* Cumulative return code will track the first error discovered */
+ if (!common->transaction->status) {
+ common->transaction->status = ret;
+ }
+
+ /* Deliver the intercepted callback prior to marking it as completed */
+ callback(cb_data->opaque, cb_data->ret);
+ transaction_job_complete(common->transaction);
+}
+
+/**
+ * Create a new transactional callback wrapper.
+ *
+ * Given a callback and a closure, generate a new
+ * callback and closure that will invoke the
+ * given callback with the given closure.
+ *
+ * After all wrappers in the transactional group have
+ * been processed, each action's .cb() method will be
+ * invoked.
+ *
+ * @common The transaction action state to set a callback for.
+ * @opaque A closure intended for the encapsulated callback.
+ * @callback The callback we are encapsulating.
+ * @new_opaque[out]: The closure to be used instead of @opaque.
+ *
+ * @return The callback to be used instead of @callback.
+ */
+__attribute__((__unused__))
+static CallbackFn *new_action_cb_wrapper(BlkActionState *common,
+ void *opaque,
+ CallbackFn *callback,
+ void **new_opaque)
+{
+ BlkActionCallbackData *cb_data = g_new0(BlkActionCallbackData, 1);
+ assert(new_opaque);
+
+ /* Stash the original callback information */
+ cb_data->opaque = opaque;
+ cb_data->callback = callback;
+ common->cb_data = cb_data;
+
+ /* The BAS itself will serve as our new closure */
+ *new_opaque = common;
+ common->refcount++;
+
+ /* Inform the transaction to expect one more return */
+ common->transaction->jobs++;
+
+ /* Lastly, the actual callback function to handle the interception. */
+ return transaction_action_callback;
+}
+
+/**
+ * Undo any actions performed by the above call.
+ */
+__attribute__((__unused__))
+static void cancel_action_cb_wrapper(BlkActionState *common)
+{
+ /* Stage 0: Wrapper was never created: */
+ if (common->cb_data == NULL) {
+ assert(common->refcount == 1);
+ return;
+ }
+
+ /* Stage 1: Callback was created, but the job never launched.
+ * (We assume that the job cannot possibly be running, because
+ * we assume it has been synchronously canceled if it was.)
+ *
+ * We also assume that any cleanup normally handled by cb()
+ * is not needed, because it should have been prepared after
+ * the job launched.
+ */
+ if (common->cb_data && common->cb_data->callback) {
+ transaction_job_cancel(common);
+ goto cleanup;
+ }
+
+ /* Stage 2: Job already completed, or was canceled.
+ *
+ * Force an error in the callback data and just invoke the completion
+ * handler to perform appropriate cleanup for us.
+ */
+ if (common->cb_data && !common->cb_data->callback) {
+ common->cb_data->ret = -ECANCELED;
+ common->ops->cb(common);
+ }
+
+ cleanup:
+ g_free(common->cb_data);
+ common->cb_data = NULL;
+ put_blk_action_state(common);
+}
+
/* internal snapshot private data */
typedef struct InternalSnapshotState {
BlkActionState common;
@@ -1927,8 +2108,10 @@ void qmp_transaction(TransactionActionList *dev_list, Error **errp)
assert(ops->instance_size > 0);
state = g_malloc0(ops->instance_size);
+ state->refcount = 1;
state->ops = ops;
state->action = dev_info;
+ state->transaction = bts;
QTAILQ_INSERT_TAIL(&bts->actions, state, entry);
state->ops->prepare(state, &local_err);
@@ -1959,10 +2142,10 @@ exit:
if (state->ops->clean) {
state->ops->clean(state);
}
- QTAILQ_REMOVE(&bts->actions, state, entry);
- g_free(state);
+ put_blk_action_state(state);
}
+ /* The implicit 'job' of the transaction itself is complete: */
transaction_job_complete(bts);
}
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH v2 06/11] block: add refcount to Job object
2015-03-27 19:19 [Qemu-devel] [PATCH v2 00/11] block: incremental backup transactions John Snow
` (4 preceding siblings ...)
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 05/11] block: add transactional callbacks feature John Snow
@ 2015-03-27 19:20 ` John Snow
2015-04-17 15:43 ` Max Reitz
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 07/11] block: add delayed bitmap successor cleanup John Snow
` (5 subsequent siblings)
11 siblings, 1 reply; 31+ messages in thread
From: John Snow @ 2015-03-27 19:20 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, mreitz, vsementsov, stefanha
If we want to get at the job after the life of the job,
we'll need a refcount for this object.
This may occur for example if we wish to inspect the actions
taken by a particular job after a transactional group of jobs
runs, and further actions are required.
Signed-off-by: John Snow <jsnow@redhat.com>
---
blockjob.c | 18 ++++++++++++++++--
include/block/blockjob.h | 21 +++++++++++++++++++++
2 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/blockjob.c b/blockjob.c
index ba2255d..d620082 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -35,6 +35,19 @@
#include "qemu/timer.h"
#include "qapi-event.h"
+void block_job_incref(BlockJob *job)
+{
+ job->refcount++;
+}
+
+void block_job_decref(BlockJob *job)
+{
+ job->refcount--;
+ if (job->refcount == 0) {
+ g_free(job);
+ }
+}
+
void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
int64_t speed, BlockCompletionFunc *cb,
void *opaque, Error **errp)
@@ -57,6 +70,7 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
job->cb = cb;
job->opaque = opaque;
job->busy = true;
+ job->refcount = 1;
bs->job = job;
/* Only set speed when necessary to avoid NotSupported error */
@@ -68,7 +82,7 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
bs->job = NULL;
bdrv_op_unblock_all(bs, job->blocker);
error_free(job->blocker);
- g_free(job);
+ block_job_decref(job);
error_propagate(errp, local_err);
return NULL;
}
@@ -85,7 +99,7 @@ void block_job_completed(BlockJob *job, int ret)
bs->job = NULL;
bdrv_op_unblock_all(bs, job->blocker);
error_free(job->blocker);
- g_free(job);
+ block_job_decref(job);
}
void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index b6d4ebb..dcc0596 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -116,6 +116,9 @@ struct BlockJob {
/** The opaque value that is passed to the completion function. */
void *opaque;
+
+ /** A reference count, allowing for post-job actions in e.g. transactions */
+ int refcount;
};
/**
@@ -141,6 +144,24 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
void *opaque, Error **errp);
/**
+ * block_job_incref:
+ * @job: The job to pick up a handle to
+ *
+ * Increment the refcount on @job, to be able to use it asynchronously
+ * from the job it is being used for. Put down the reference when done
+ * with @block_job_unref.
+ */
+void block_job_incref(BlockJob *job);
+
+/**
+ * block_job_decref:
+ * @job: The job to unreference and delete.
+ *
+ * Decrement the refcount on @job and delete it if there are no more references.
+ */
+void block_job_decref(BlockJob *job);
+
+/**
* block_job_sleep_ns:
* @job: The job that calls the function.
* @clock: The clock to sleep on.
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH v2 07/11] block: add delayed bitmap successor cleanup
2015-03-27 19:19 [Qemu-devel] [PATCH v2 00/11] block: incremental backup transactions John Snow
` (5 preceding siblings ...)
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 06/11] block: add refcount to Job object John Snow
@ 2015-03-27 19:20 ` John Snow
2015-04-17 15:49 ` Max Reitz
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 08/11] block: move transactions beneath qmp interfaces John Snow
` (4 subsequent siblings)
11 siblings, 1 reply; 31+ messages in thread
From: John Snow @ 2015-03-27 19:20 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, mreitz, vsementsov, stefanha
Allow bitmap successors to carry reference counts.
We can in a later patch use this ability to clean up the dirty bitmap
according to both the individual job's success and the success of all
jobs in the transaction group.
The code for cleaning up a bitmap is also moved from backup_run to
backup_complete.
Signed-off-by: John Snow <jsnow@redhat.com>
---
block.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++-----
block/backup.c | 20 ++++++----------
include/block/block.h | 10 ++++----
3 files changed, 70 insertions(+), 25 deletions(-)
diff --git a/block.c b/block.c
index 42839a0..a1d01ba 100644
--- a/block.c
+++ b/block.c
@@ -51,6 +51,12 @@
#include <windows.h>
#endif
+typedef enum BitmapSuccessorAction {
+ SUCCESSOR_ACTION_UNDEFINED = 0,
+ SUCCESSOR_ACTION_ABDICATE,
+ SUCCESSOR_ACTION_RECLAIM
+} BitmapSuccessorAction;
+
/**
* A BdrvDirtyBitmap can be in three possible states:
* (1) successor is false and disabled is false: full r/w mode
@@ -65,6 +71,8 @@ struct BdrvDirtyBitmap {
char *name; /* Optional non-empty unique ID */
int64_t size; /* Size of the bitmap (Number of sectors) */
bool disabled; /* Bitmap is read-only */
+ int successor_refcount; /* Number of active handles to the successor */
+ BitmapSuccessorAction act; /* Action to take on successor upon release */
QLIST_ENTRY(BdrvDirtyBitmap) list;
};
@@ -5540,6 +5548,7 @@ int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
/* Install the successor and freeze the parent */
bitmap->successor = child;
+ bitmap->successor_refcount = 1;
return 0;
}
@@ -5547,9 +5556,9 @@ int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
* For a bitmap with a successor, yield our name to the successor,
* Delete the old bitmap, and return a handle to the new bitmap.
*/
-BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
- BdrvDirtyBitmap *bitmap,
- Error **errp)
+static BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap,
+ Error **errp)
{
char *name;
BdrvDirtyBitmap *successor = bitmap->successor;
@@ -5574,9 +5583,9 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
* We may wish to re-join the parent and child/successor.
* The merged parent will be un-frozen, but not explicitly re-enabled.
*/
-BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
- BdrvDirtyBitmap *parent,
- Error **errp)
+static BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
+ BdrvDirtyBitmap *parent,
+ Error **errp)
{
BdrvDirtyBitmap *successor = parent->successor;
@@ -5595,6 +5604,50 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
return parent;
}
+static BdrvDirtyBitmap *bdrv_free_bitmap_successor(BlockDriverState *bs,
+ BdrvDirtyBitmap *parent)
+{
+ assert(!parent->successor_refcount);
+
+ switch (parent->act) {
+ case SUCCESSOR_ACTION_RECLAIM:
+ return bdrv_reclaim_dirty_bitmap(bs, parent, NULL);
+ case SUCCESSOR_ACTION_ABDICATE:
+ return bdrv_dirty_bitmap_abdicate(bs, parent, NULL);
+ case SUCCESSOR_ACTION_UNDEFINED:
+ default:
+ g_assert_not_reached();
+ }
+}
+
+BdrvDirtyBitmap *bdrv_frozen_bitmap_decref(BlockDriverState *bs,
+ BdrvDirtyBitmap *parent,
+ int ret)
+{
+ assert(bdrv_dirty_bitmap_frozen(parent));
+ assert(parent->successor);
+
+ if (ret) {
+ parent->act = SUCCESSOR_ACTION_RECLAIM;
+ } else if (parent->act != SUCCESSOR_ACTION_RECLAIM) {
+ parent->act = SUCCESSOR_ACTION_ABDICATE;
+ }
+
+ parent->successor_refcount--;
+ if (parent->successor_refcount == 0) {
+ return bdrv_free_bitmap_successor(bs, parent);
+ }
+ return parent;
+}
+
+void bdrv_dirty_bitmap_incref(BdrvDirtyBitmap *parent)
+{
+ assert(bdrv_dirty_bitmap_frozen(parent));
+ assert(parent->successor);
+
+ parent->successor_refcount++;
+}
+
/**
* Truncates _all_ bitmaps attached to a BDS.
*/
diff --git a/block/backup.c b/block/backup.c
index 4f9a3de..1fefca7 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -240,6 +240,12 @@ static void backup_complete(BlockJob *job, void *opaque)
bdrv_unref(s->target);
+ if (s->sync_bitmap) {
+ BdrvDirtyBitmap *bm;
+ bm = bdrv_frozen_bitmap_decref(job->bs, s->sync_bitmap, data->ret);
+ assert(bm);
+ }
+
block_job_completed(job, data->ret);
g_free(data);
}
@@ -419,18 +425,6 @@ leave:
qemu_co_rwlock_wrlock(&job->flush_rwlock);
qemu_co_rwlock_unlock(&job->flush_rwlock);
- if (job->sync_bitmap) {
- BdrvDirtyBitmap *bm;
- if (ret < 0) {
- /* Merge the successor back into the parent, delete nothing. */
- bm = bdrv_reclaim_dirty_bitmap(bs, job->sync_bitmap, NULL);
- assert(bm);
- } else {
- /* Everything is fine, delete this bitmap and install the backup. */
- bm = bdrv_dirty_bitmap_abdicate(bs, job->sync_bitmap, NULL);
- assert(bm);
- }
- }
hbitmap_free(job->bitmap);
bdrv_iostatus_disable(target);
@@ -534,6 +528,6 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
error:
if (sync_bitmap) {
- bdrv_reclaim_dirty_bitmap(bs, sync_bitmap, NULL);
+ bdrv_frozen_bitmap_decref(bs, sync_bitmap, -ECANCELED);
}
}
diff --git a/include/block/block.h b/include/block/block.h
index 36bdf04..7b548b1 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -456,12 +456,10 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap,
Error **errp);
-BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
- BdrvDirtyBitmap *bitmap,
- Error **errp);
-BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
- BdrvDirtyBitmap *bitmap,
- Error **errp);
+BdrvDirtyBitmap *bdrv_frozen_bitmap_decref(BlockDriverState *bs,
+ BdrvDirtyBitmap *parent,
+ int ret);
+void bdrv_dirty_bitmap_incref(BdrvDirtyBitmap *parent);
BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
const char *name);
void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap);
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH v2 08/11] block: move transactions beneath qmp interfaces
2015-03-27 19:19 [Qemu-devel] [PATCH v2 00/11] block: incremental backup transactions John Snow
` (6 preceding siblings ...)
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 07/11] block: add delayed bitmap successor cleanup John Snow
@ 2015-03-27 19:20 ` John Snow
2015-04-17 16:01 ` Max Reitz
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 09/11] qmp: Add an implementation wrapper for qmp_drive_backup John Snow
` (3 subsequent siblings)
11 siblings, 1 reply; 31+ messages in thread
From: John Snow @ 2015-03-27 19:20 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, mreitz, vsementsov, stefanha
In general, since transactions may reference QMP function helpers,
it would be nice for them to sit beneath them.
This will avoid the need for forward declaring any QMP interfaces,
which would be aggravating to update in so many places.
Signed-off-by: John Snow <jsnow@redhat.com>
---
blockdev.c | 2581 ++++++++++++++++++++++++++++++------------------------------
1 file changed, 1292 insertions(+), 1289 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index d404251..fa954e9 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1225,7 +1225,1297 @@ static BdrvDirtyBitmap *block_dirty_bitmap_lookup(const char *node,
return NULL;
}
-/* New and old BlockDriverState structs for atomic group operations */
+static void eject_device(BlockBackend *blk, int force, Error **errp)
+{
+ BlockDriverState *bs = blk_bs(blk);
+ AioContext *aio_context;
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) {
+ goto out;
+ }
+ if (!blk_dev_has_removable_media(blk)) {
+ error_setg(errp, "Device '%s' is not removable",
+ bdrv_get_device_name(bs));
+ goto out;
+ }
+
+ if (blk_dev_is_medium_locked(blk) && !blk_dev_is_tray_open(blk)) {
+ blk_dev_eject_request(blk, force);
+ if (!force) {
+ error_setg(errp, "Device '%s' is locked",
+ bdrv_get_device_name(bs));
+ goto out;
+ }
+ }
+
+ bdrv_close(bs);
+
+out:
+ aio_context_release(aio_context);
+}
+
+void qmp_eject(const char *device, bool has_force, bool force, Error **errp)
+{
+ BlockBackend *blk;
+
+ blk = blk_by_name(device);
+ if (!blk) {
+ error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+ return;
+ }
+
+ eject_device(blk, force, errp);
+}
+
+void qmp_block_passwd(bool has_device, const char *device,
+ bool has_node_name, const char *node_name,
+ const char *password, Error **errp)
+{
+ Error *local_err = NULL;
+ BlockDriverState *bs;
+ AioContext *aio_context;
+
+ bs = bdrv_lookup_bs(has_device ? device : NULL,
+ has_node_name ? node_name : NULL,
+ &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ bdrv_add_key(bs, password, errp);
+
+ aio_context_release(aio_context);
+}
+
+/* Assumes AioContext is held */
+static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
+ int bdrv_flags, BlockDriver *drv,
+ const char *password, Error **errp)
+{
+ Error *local_err = NULL;
+ int ret;
+
+ ret = bdrv_open(&bs, filename, NULL, NULL, bdrv_flags, drv, &local_err);
+ if (ret < 0) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ bdrv_add_key(bs, password, errp);
+}
+
+void qmp_change_blockdev(const char *device, const char *filename,
+ const char *format, Error **errp)
+{
+ BlockBackend *blk;
+ BlockDriverState *bs;
+ AioContext *aio_context;
+ BlockDriver *drv = NULL;
+ int bdrv_flags;
+ Error *err = NULL;
+
+ blk = blk_by_name(device);
+ if (!blk) {
+ error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+ return;
+ }
+ bs = blk_bs(blk);
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ if (format) {
+ drv = bdrv_find_whitelisted_format(format, bs->read_only);
+ if (!drv) {
+ error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
+ goto out;
+ }
+ }
+
+ eject_device(blk, 0, &err);
+ if (err) {
+ error_propagate(errp, err);
+ goto out;
+ }
+
+ bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
+ bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
+
+ qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, errp);
+
+out:
+ aio_context_release(aio_context);
+}
+
+/* throttling disk I/O limits */
+void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
+ int64_t bps_wr,
+ int64_t iops,
+ int64_t iops_rd,
+ int64_t iops_wr,
+ bool has_bps_max,
+ int64_t bps_max,
+ bool has_bps_rd_max,
+ int64_t bps_rd_max,
+ bool has_bps_wr_max,
+ int64_t bps_wr_max,
+ bool has_iops_max,
+ int64_t iops_max,
+ bool has_iops_rd_max,
+ int64_t iops_rd_max,
+ bool has_iops_wr_max,
+ int64_t iops_wr_max,
+ bool has_iops_size,
+ int64_t iops_size, Error **errp)
+{
+ ThrottleConfig cfg;
+ BlockDriverState *bs;
+ BlockBackend *blk;
+ AioContext *aio_context;
+
+ blk = blk_by_name(device);
+ if (!blk) {
+ error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+ return;
+ }
+ bs = blk_bs(blk);
+
+ memset(&cfg, 0, sizeof(cfg));
+ cfg.buckets[THROTTLE_BPS_TOTAL].avg = bps;
+ cfg.buckets[THROTTLE_BPS_READ].avg = bps_rd;
+ cfg.buckets[THROTTLE_BPS_WRITE].avg = bps_wr;
+
+ cfg.buckets[THROTTLE_OPS_TOTAL].avg = iops;
+ cfg.buckets[THROTTLE_OPS_READ].avg = iops_rd;
+ cfg.buckets[THROTTLE_OPS_WRITE].avg = iops_wr;
+
+ if (has_bps_max) {
+ cfg.buckets[THROTTLE_BPS_TOTAL].max = bps_max;
+ }
+ if (has_bps_rd_max) {
+ cfg.buckets[THROTTLE_BPS_READ].max = bps_rd_max;
+ }
+ if (has_bps_wr_max) {
+ cfg.buckets[THROTTLE_BPS_WRITE].max = bps_wr_max;
+ }
+ if (has_iops_max) {
+ cfg.buckets[THROTTLE_OPS_TOTAL].max = iops_max;
+ }
+ if (has_iops_rd_max) {
+ cfg.buckets[THROTTLE_OPS_READ].max = iops_rd_max;
+ }
+ if (has_iops_wr_max) {
+ cfg.buckets[THROTTLE_OPS_WRITE].max = iops_wr_max;
+ }
+
+ if (has_iops_size) {
+ cfg.op_size = iops_size;
+ }
+
+ if (!check_throttle_config(&cfg, errp)) {
+ return;
+ }
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ if (!bs->io_limits_enabled && throttle_enabled(&cfg)) {
+ bdrv_io_limits_enable(bs);
+ } else if (bs->io_limits_enabled && !throttle_enabled(&cfg)) {
+ bdrv_io_limits_disable(bs);
+ }
+
+ if (bs->io_limits_enabled) {
+ bdrv_set_io_limits(bs, &cfg);
+ }
+
+ aio_context_release(aio_context);
+}
+
+void qmp_block_dirty_bitmap_add(const char *node, const char *name,
+ bool has_granularity, uint32_t granularity,
+ Error **errp)
+{
+ AioContext *aio_context;
+ BlockDriverState *bs;
+
+ if (!name || name[0] == '\0') {
+ error_setg(errp, "Bitmap name cannot be empty");
+ return;
+ }
+
+ bs = bdrv_lookup_bs(node, node, errp);
+ if (!bs) {
+ return;
+ }
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ if (has_granularity) {
+ if (granularity < 512 || !is_power_of_2(granularity)) {
+ error_setg(errp, "Granularity must be power of 2 "
+ "and at least 512");
+ goto out;
+ }
+ } else {
+ /* Default to cluster size, if available: */
+ granularity = bdrv_get_default_bitmap_granularity(bs);
+ }
+
+ bdrv_create_dirty_bitmap(bs, granularity, name, errp);
+
+ out:
+ aio_context_release(aio_context);
+}
+
+void qmp_block_dirty_bitmap_remove(const char *node, const char *name,
+ Error **errp)
+{
+ AioContext *aio_context;
+ BlockDriverState *bs;
+ BdrvDirtyBitmap *bitmap;
+
+ bitmap = block_dirty_bitmap_lookup(node, name, &bs, &aio_context, errp);
+ if (!bitmap || !bs) {
+ return;
+ }
+
+ if (bdrv_dirty_bitmap_frozen(bitmap)) {
+ error_setg(errp,
+ "Bitmap '%s' is currently frozen and cannot be removed",
+ name);
+ goto out;
+ }
+ bdrv_dirty_bitmap_make_anon(bitmap);
+ bdrv_release_dirty_bitmap(bs, bitmap);
+
+ out:
+ aio_context_release(aio_context);
+}
+
+/**
+ * Completely clear a bitmap, for the purposes of synchronizing a bitmap
+ * immediately after a full backup operation.
+ */
+void qmp_block_dirty_bitmap_clear(const char *node, const char *name,
+ Error **errp)
+{
+ AioContext *aio_context;
+ BdrvDirtyBitmap *bitmap;
+ BlockDriverState *bs;
+
+ bitmap = block_dirty_bitmap_lookup(node, name, &bs, &aio_context, errp);
+ if (!bitmap || !bs) {
+ return;
+ }
+
+ if (bdrv_dirty_bitmap_frozen(bitmap)) {
+ error_setg(errp,
+ "Bitmap '%s' is currently frozen and cannot be modified",
+ name);
+ goto out;
+ } else if (!bdrv_dirty_bitmap_enabled(bitmap)) {
+ error_setg(errp,
+ "Bitmap '%s' is currently disabled and cannot be cleared",
+ name);
+ goto out;
+ }
+
+ bdrv_clear_dirty_bitmap(bitmap);
+
+ out:
+ aio_context_release(aio_context);
+}
+
+int hmp_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+ const char *id = qdict_get_str(qdict, "id");
+ BlockBackend *blk;
+ BlockDriverState *bs;
+ AioContext *aio_context;
+ Error *local_err = NULL;
+
+ blk = blk_by_name(id);
+ if (!blk) {
+ error_report("Device '%s' not found", id);
+ return -1;
+ }
+ bs = blk_bs(blk);
+
+ if (!blk_legacy_dinfo(blk)) {
+ error_report("Deleting device added with blockdev-add"
+ " is not supported");
+ return -1;
+ }
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, &local_err)) {
+ error_report_err(local_err);
+ aio_context_release(aio_context);
+ return -1;
+ }
+
+ /* quiesce block driver; prevent further io */
+ bdrv_drain_all();
+ bdrv_flush(bs);
+ bdrv_close(bs);
+
+ /* if we have a device attached to this BlockDriverState
+ * then we need to make the drive anonymous until the device
+ * can be removed. If this is a drive with no device backing
+ * then we can just get rid of the block driver state right here.
+ */
+ if (blk_get_attached_dev(blk)) {
+ blk_hide_on_behalf_of_hmp_drive_del(blk);
+ /* Further I/O must not pause the guest */
+ bdrv_set_on_error(bs, BLOCKDEV_ON_ERROR_REPORT,
+ BLOCKDEV_ON_ERROR_REPORT);
+ } else {
+ blk_unref(blk);
+ }
+
+ aio_context_release(aio_context);
+ return 0;
+}
+
+void qmp_block_resize(bool has_device, const char *device,
+ bool has_node_name, const char *node_name,
+ int64_t size, Error **errp)
+{
+ Error *local_err = NULL;
+ BlockDriverState *bs;
+ AioContext *aio_context;
+ int ret;
+
+ bs = bdrv_lookup_bs(has_device ? device : NULL,
+ has_node_name ? node_name : NULL,
+ &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ if (!bdrv_is_first_non_filter(bs)) {
+ error_set(errp, QERR_FEATURE_DISABLED, "resize");
+ goto out;
+ }
+
+ if (size < 0) {
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, "size", "a >0 size");
+ goto out;
+ }
+
+ if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) {
+ error_set(errp, QERR_DEVICE_IN_USE, device);
+ goto out;
+ }
+
+ /* complete all in-flight operations before resizing the device */
+ bdrv_drain_all();
+
+ ret = bdrv_truncate(bs, size);
+ switch (ret) {
+ case 0:
+ break;
+ case -ENOMEDIUM:
+ error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
+ break;
+ case -ENOTSUP:
+ error_set(errp, QERR_UNSUPPORTED);
+ break;
+ case -EACCES:
+ error_set(errp, QERR_DEVICE_IS_READ_ONLY, device);
+ break;
+ case -EBUSY:
+ error_set(errp, QERR_DEVICE_IN_USE, device);
+ break;
+ default:
+ error_setg_errno(errp, -ret, "Could not resize");
+ break;
+ }
+
+out:
+ aio_context_release(aio_context);
+}
+
+static void block_job_cb(void *opaque, int ret)
+{
+ /* Note that this function may be executed from another AioContext besides
+ * the QEMU main loop. If you need to access anything that assumes the
+ * QEMU global mutex, use a BH or introduce a mutex.
+ */
+
+ BlockDriverState *bs = opaque;
+ const char *msg = NULL;
+
+ trace_block_job_cb(bs, bs->job, ret);
+
+ assert(bs->job);
+
+ if (ret < 0) {
+ msg = strerror(-ret);
+ }
+
+ if (block_job_is_cancelled(bs->job)) {
+ block_job_event_cancelled(bs->job);
+ } else {
+ block_job_event_completed(bs->job, msg);
+ }
+
+ bdrv_put_ref_bh_schedule(bs);
+}
+
+void qmp_block_stream(const char *device,
+ bool has_base, const char *base,
+ bool has_backing_file, const char *backing_file,
+ bool has_speed, int64_t speed,
+ bool has_on_error, BlockdevOnError on_error,
+ Error **errp)
+{
+ BlockBackend *blk;
+ BlockDriverState *bs;
+ BlockDriverState *base_bs = NULL;
+ AioContext *aio_context;
+ Error *local_err = NULL;
+ const char *base_name = NULL;
+
+ if (!has_on_error) {
+ on_error = BLOCKDEV_ON_ERROR_REPORT;
+ }
+
+ blk = blk_by_name(device);
+ if (!blk) {
+ error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+ return;
+ }
+ bs = blk_bs(blk);
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_STREAM, errp)) {
+ goto out;
+ }
+
+ if (has_base) {
+ base_bs = bdrv_find_backing_image(bs, base);
+ if (base_bs == NULL) {
+ error_set(errp, QERR_BASE_NOT_FOUND, base);
+ goto out;
+ }
+ assert(bdrv_get_aio_context(base_bs) == aio_context);
+ base_name = base;
+ }
+
+ /* if we are streaming the entire chain, the result will have no backing
+ * file, and specifying one is therefore an error */
+ if (base_bs == NULL && has_backing_file) {
+ error_setg(errp, "backing file specified, but streaming the "
+ "entire chain");
+ goto out;
+ }
+
+ /* backing_file string overrides base bs filename */
+ base_name = has_backing_file ? backing_file : base_name;
+
+ stream_start(bs, base_bs, base_name, has_speed ? speed : 0,
+ on_error, block_job_cb, bs, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ goto out;
+ }
+
+ trace_qmp_block_stream(bs, bs->job);
+
+out:
+ aio_context_release(aio_context);
+}
+
+void qmp_block_commit(const char *device,
+ bool has_base, const char *base,
+ bool has_top, const char *top,
+ bool has_backing_file, const char *backing_file,
+ bool has_speed, int64_t speed,
+ Error **errp)
+{
+ BlockBackend *blk;
+ BlockDriverState *bs;
+ BlockDriverState *base_bs, *top_bs;
+ AioContext *aio_context;
+ Error *local_err = NULL;
+ /* This will be part of the QMP command, if/when the
+ * BlockdevOnError change for blkmirror makes it in
+ */
+ BlockdevOnError on_error = BLOCKDEV_ON_ERROR_REPORT;
+
+ if (!has_speed) {
+ speed = 0;
+ }
+
+ /* Important Note:
+ * libvirt relies on the DeviceNotFound error class in order to probe for
+ * live commit feature versions; for this to work, we must make sure to
+ * perform the device lookup before any generic errors that may occur in a
+ * scenario in which all optional arguments are omitted. */
+ blk = blk_by_name(device);
+ if (!blk) {
+ error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+ return;
+ }
+ bs = blk_bs(blk);
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ /* drain all i/o before commits */
+ bdrv_drain_all();
+
+ if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT_SOURCE, errp)) {
+ goto out;
+ }
+
+ /* default top_bs is the active layer */
+ top_bs = bs;
+
+ if (has_top && top) {
+ if (strcmp(bs->filename, top) != 0) {
+ top_bs = bdrv_find_backing_image(bs, top);
+ }
+ }
+
+ if (top_bs == NULL) {
+ error_setg(errp, "Top image file %s not found", top ? top : "NULL");
+ goto out;
+ }
+
+ assert(bdrv_get_aio_context(top_bs) == aio_context);
+
+ if (has_base && base) {
+ base_bs = bdrv_find_backing_image(top_bs, base);
+ } else {
+ base_bs = bdrv_find_base(top_bs);
+ }
+
+ if (base_bs == NULL) {
+ error_set(errp, QERR_BASE_NOT_FOUND, base ? base : "NULL");
+ goto out;
+ }
+
+ assert(bdrv_get_aio_context(base_bs) == aio_context);
+
+ if (bdrv_op_is_blocked(base_bs, BLOCK_OP_TYPE_COMMIT_TARGET, errp)) {
+ goto out;
+ }
+
+ /* Do not allow attempts to commit an image into itself */
+ if (top_bs == base_bs) {
+ error_setg(errp, "cannot commit an image into itself");
+ goto out;
+ }
+
+ if (top_bs == bs) {
+ if (has_backing_file) {
+ error_setg(errp, "'backing-file' specified,"
+ " but 'top' is the active layer");
+ goto out;
+ }
+ commit_active_start(bs, base_bs, speed, on_error, block_job_cb,
+ bs, &local_err);
+ } else {
+ commit_start(bs, base_bs, top_bs, speed, on_error, block_job_cb, bs,
+ has_backing_file ? backing_file : NULL, &local_err);
+ }
+ if (local_err != NULL) {
+ error_propagate(errp, local_err);
+ goto out;
+ }
+
+out:
+ aio_context_release(aio_context);
+}
+
+void qmp_drive_backup(const char *device, const char *target,
+ bool has_format, const char *format,
+ enum MirrorSyncMode sync,
+ bool has_mode, enum NewImageMode mode,
+ bool has_speed, int64_t speed,
+ bool has_bitmap, const char *bitmap,
+ bool has_on_source_error, BlockdevOnError on_source_error,
+ bool has_on_target_error, BlockdevOnError on_target_error,
+ Error **errp)
+{
+ BlockBackend *blk;
+ BlockDriverState *bs;
+ BlockDriverState *target_bs;
+ BlockDriverState *source = NULL;
+ BdrvDirtyBitmap *bmap = NULL;
+ AioContext *aio_context;
+ BlockDriver *drv = NULL;
+ Error *local_err = NULL;
+ int flags;
+ int64_t size;
+ int ret;
+
+ if (!has_speed) {
+ speed = 0;
+ }
+ if (!has_on_source_error) {
+ on_source_error = BLOCKDEV_ON_ERROR_REPORT;
+ }
+ if (!has_on_target_error) {
+ on_target_error = BLOCKDEV_ON_ERROR_REPORT;
+ }
+ if (!has_mode) {
+ mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
+ }
+
+ blk = blk_by_name(device);
+ if (!blk) {
+ error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+ return;
+ }
+ bs = blk_bs(blk);
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ /* Although backup_run has this check too, we need to use bs->drv below, so
+ * do an early check redundantly. */
+ if (!bdrv_is_inserted(bs)) {
+ error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
+ goto out;
+ }
+
+ if (!has_format) {
+ format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
+ }
+ if (format) {
+ drv = bdrv_find_format(format);
+ if (!drv) {
+ error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
+ goto out;
+ }
+ }
+
+ /* Early check to avoid creating target */
+ if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
+ goto out;
+ }
+
+ flags = bs->open_flags | BDRV_O_RDWR;
+
+ /* See if we have a backing HD we can use to create our new image
+ * on top of. */
+ if (sync == MIRROR_SYNC_MODE_TOP) {
+ source = bs->backing_hd;
+ if (!source) {
+ sync = MIRROR_SYNC_MODE_FULL;
+ }
+ }
+ if (sync == MIRROR_SYNC_MODE_NONE) {
+ source = bs;
+ }
+
+ size = bdrv_getlength(bs);
+ if (size < 0) {
+ error_setg_errno(errp, -size, "bdrv_getlength failed");
+ goto out;
+ }
+
+ if (mode != NEW_IMAGE_MODE_EXISTING) {
+ assert(format && drv);
+ if (source) {
+ bdrv_img_create(target, format, source->filename,
+ source->drv->format_name, NULL,
+ size, flags, &local_err, false);
+ } else {
+ bdrv_img_create(target, format, NULL, NULL, NULL,
+ size, flags, &local_err, false);
+ }
+ }
+
+ if (local_err) {
+ error_propagate(errp, local_err);
+ goto out;
+ }
+
+ target_bs = NULL;
+ ret = bdrv_open(&target_bs, target, NULL, NULL, flags, drv, &local_err);
+ if (ret < 0) {
+ error_propagate(errp, local_err);
+ goto out;
+ }
+
+ bdrv_set_aio_context(target_bs, aio_context);
+
+ if (has_bitmap) {
+ bmap = bdrv_find_dirty_bitmap(bs, bitmap);
+ if (!bmap) {
+ error_setg(errp, "Bitmap '%s' could not be found", bitmap);
+ goto out;
+ }
+ }
+
+ backup_start(bs, target_bs, speed, sync, bmap,
+ on_source_error, on_target_error,
+ block_job_cb, bs, &local_err);
+ if (local_err != NULL) {
+ bdrv_unref(target_bs);
+ error_propagate(errp, local_err);
+ goto out;
+ }
+
+out:
+ aio_context_release(aio_context);
+}
+
+BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp)
+{
+ return bdrv_named_nodes_list();
+}
+
+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,
+ Error **errp)
+{
+ BlockBackend *blk;
+ BlockDriverState *bs;
+ BlockDriverState *target_bs;
+ Error *local_err = NULL;
+ AioContext *aio_context;
+
+ if (!has_speed) {
+ speed = 0;
+ }
+ if (!has_on_source_error) {
+ on_source_error = BLOCKDEV_ON_ERROR_REPORT;
+ }
+ if (!has_on_target_error) {
+ on_target_error = BLOCKDEV_ON_ERROR_REPORT;
+ }
+
+ blk = blk_by_name(device);
+ if (!blk) {
+ error_setg(errp, "Device '%s' not found", device);
+ return;
+ }
+ bs = blk_bs(blk);
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ blk = blk_by_name(target);
+ if (!blk) {
+ error_setg(errp, "Device '%s' not found", target);
+ goto out;
+ }
+ target_bs = blk_bs(blk);
+
+ bdrv_ref(target_bs);
+ bdrv_set_aio_context(target_bs, aio_context);
+ backup_start(bs, target_bs, speed, sync, NULL, on_source_error,
+ on_target_error, block_job_cb, bs, &local_err);
+ if (local_err != NULL) {
+ bdrv_unref(target_bs);
+ error_propagate(errp, local_err);
+ }
+out:
+ aio_context_release(aio_context);
+}
+
+#define DEFAULT_MIRROR_BUF_SIZE (10 << 20)
+
+void qmp_drive_mirror(const char *device, const char *target,
+ bool has_format, const char *format,
+ bool has_node_name, const char *node_name,
+ bool has_replaces, const char *replaces,
+ enum MirrorSyncMode sync,
+ bool has_mode, enum NewImageMode mode,
+ bool has_speed, int64_t speed,
+ bool has_granularity, uint32_t granularity,
+ bool has_buf_size, int64_t buf_size,
+ bool has_on_source_error, BlockdevOnError on_source_error,
+ bool has_on_target_error, BlockdevOnError on_target_error,
+ Error **errp)
+{
+ BlockBackend *blk;
+ BlockDriverState *bs;
+ BlockDriverState *source, *target_bs;
+ AioContext *aio_context;
+ BlockDriver *drv = NULL;
+ Error *local_err = NULL;
+ QDict *options = NULL;
+ int flags;
+ int64_t size;
+ int ret;
+
+ if (!has_speed) {
+ speed = 0;
+ }
+ if (!has_on_source_error) {
+ on_source_error = BLOCKDEV_ON_ERROR_REPORT;
+ }
+ if (!has_on_target_error) {
+ on_target_error = BLOCKDEV_ON_ERROR_REPORT;
+ }
+ if (!has_mode) {
+ mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
+ }
+ if (!has_granularity) {
+ granularity = 0;
+ }
+ if (!has_buf_size) {
+ buf_size = DEFAULT_MIRROR_BUF_SIZE;
+ }
+
+ if (granularity != 0 && (granularity < 512 || granularity > 1048576 * 64)) {
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, "granularity",
+ "a value in range [512B, 64MB]");
+ return;
+ }
+ if (granularity & (granularity - 1)) {
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE,
+ "granularity", "power of 2");
+ return;
+ }
+
+ blk = blk_by_name(device);
+ if (!blk) {
+ error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+ return;
+ }
+ bs = blk_bs(blk);
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ if (!bdrv_is_inserted(bs)) {
+ error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
+ goto out;
+ }
+
+ if (!has_format) {
+ format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
+ }
+ if (format) {
+ drv = bdrv_find_format(format);
+ if (!drv) {
+ error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
+ goto out;
+ }
+ }
+
+ if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR, errp)) {
+ goto out;
+ }
+
+ flags = bs->open_flags | BDRV_O_RDWR;
+ source = bs->backing_hd;
+ if (!source && sync == MIRROR_SYNC_MODE_TOP) {
+ sync = MIRROR_SYNC_MODE_FULL;
+ }
+ if (sync == MIRROR_SYNC_MODE_NONE) {
+ source = bs;
+ }
+
+ size = bdrv_getlength(bs);
+ if (size < 0) {
+ error_setg_errno(errp, -size, "bdrv_getlength failed");
+ goto out;
+ }
+
+ if (has_replaces) {
+ BlockDriverState *to_replace_bs;
+ AioContext *replace_aio_context;
+ int64_t replace_size;
+
+ if (!has_node_name) {
+ error_setg(errp, "a node-name must be provided when replacing a"
+ " named node of the graph");
+ goto out;
+ }
+
+ to_replace_bs = check_to_replace_node(replaces, &local_err);
+
+ if (!to_replace_bs) {
+ error_propagate(errp, local_err);
+ goto out;
+ }
+
+ replace_aio_context = bdrv_get_aio_context(to_replace_bs);
+ aio_context_acquire(replace_aio_context);
+ replace_size = bdrv_getlength(to_replace_bs);
+ aio_context_release(replace_aio_context);
+
+ if (size != replace_size) {
+ error_setg(errp, "cannot replace image with a mirror image of "
+ "different size");
+ goto out;
+ }
+ }
+
+ if ((sync == MIRROR_SYNC_MODE_FULL || !source)
+ && mode != NEW_IMAGE_MODE_EXISTING)
+ {
+ /* create new image w/o backing file */
+ assert(format && drv);
+ bdrv_img_create(target, format,
+ NULL, NULL, NULL, size, flags, &local_err, false);
+ } else {
+ switch (mode) {
+ case NEW_IMAGE_MODE_EXISTING:
+ break;
+ case NEW_IMAGE_MODE_ABSOLUTE_PATHS:
+ /* create new image with backing file */
+ bdrv_img_create(target, format,
+ source->filename,
+ source->drv->format_name,
+ NULL, size, flags, &local_err, false);
+ break;
+ default:
+ abort();
+ }
+ }
+
+ if (local_err) {
+ error_propagate(errp, local_err);
+ goto out;
+ }
+
+ if (has_node_name) {
+ options = qdict_new();
+ qdict_put(options, "node-name", qstring_from_str(node_name));
+ }
+
+ /* Mirroring takes care of copy-on-write using the source's backing
+ * file.
+ */
+ target_bs = NULL;
+ ret = bdrv_open(&target_bs, target, NULL, options,
+ flags | BDRV_O_NO_BACKING, drv, &local_err);
+ if (ret < 0) {
+ error_propagate(errp, local_err);
+ goto out;
+ }
+
+ bdrv_set_aio_context(target_bs, aio_context);
+
+ /* pass the node name to replace to mirror start since it's loose coupling
+ * and will allow to check whether the node still exist at mirror completion
+ */
+ mirror_start(bs, target_bs,
+ has_replaces ? replaces : NULL,
+ speed, granularity, buf_size, sync,
+ on_source_error, on_target_error,
+ block_job_cb, bs, &local_err);
+ if (local_err != NULL) {
+ bdrv_unref(target_bs);
+ error_propagate(errp, local_err);
+ goto out;
+ }
+
+out:
+ aio_context_release(aio_context);
+}
+
+/* Get the block job for a given device name and acquire its AioContext */
+static BlockJob *find_block_job(const char *device, AioContext **aio_context,
+ Error **errp)
+{
+ BlockBackend *blk;
+ BlockDriverState *bs;
+
+ blk = blk_by_name(device);
+ if (!blk) {
+ goto notfound;
+ }
+ bs = blk_bs(blk);
+
+ *aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(*aio_context);
+
+ if (!bs->job) {
+ aio_context_release(*aio_context);
+ goto notfound;
+ }
+
+ return bs->job;
+
+notfound:
+ error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE,
+ "No active block job on device '%s'", device);
+ *aio_context = NULL;
+ return NULL;
+}
+
+void qmp_block_job_set_speed(const char *device, int64_t speed, Error **errp)
+{
+ AioContext *aio_context;
+ BlockJob *job = find_block_job(device, &aio_context, errp);
+
+ if (!job) {
+ return;
+ }
+
+ block_job_set_speed(job, speed, errp);
+ aio_context_release(aio_context);
+}
+
+void qmp_block_job_cancel(const char *device,
+ bool has_force, bool force, Error **errp)
+{
+ AioContext *aio_context;
+ BlockJob *job = find_block_job(device, &aio_context, errp);
+
+ if (!job) {
+ return;
+ }
+
+ if (!has_force) {
+ force = false;
+ }
+
+ if (job->paused && !force) {
+ error_setg(errp, "The block job for device '%s' is currently paused",
+ device);
+ goto out;
+ }
+
+ trace_qmp_block_job_cancel(job);
+ block_job_cancel(job);
+out:
+ aio_context_release(aio_context);
+}
+
+void qmp_block_job_pause(const char *device, Error **errp)
+{
+ AioContext *aio_context;
+ BlockJob *job = find_block_job(device, &aio_context, errp);
+
+ if (!job) {
+ return;
+ }
+
+ trace_qmp_block_job_pause(job);
+ block_job_pause(job);
+ aio_context_release(aio_context);
+}
+
+void qmp_block_job_resume(const char *device, Error **errp)
+{
+ AioContext *aio_context;
+ BlockJob *job = find_block_job(device, &aio_context, errp);
+
+ if (!job) {
+ return;
+ }
+
+ trace_qmp_block_job_resume(job);
+ block_job_resume(job);
+ aio_context_release(aio_context);
+}
+
+void qmp_block_job_complete(const char *device, Error **errp)
+{
+ AioContext *aio_context;
+ BlockJob *job = find_block_job(device, &aio_context, errp);
+
+ if (!job) {
+ return;
+ }
+
+ trace_qmp_block_job_complete(job);
+ block_job_complete(job, errp);
+ aio_context_release(aio_context);
+}
+
+void qmp_change_backing_file(const char *device,
+ const char *image_node_name,
+ const char *backing_file,
+ Error **errp)
+{
+ BlockBackend *blk;
+ BlockDriverState *bs = NULL;
+ AioContext *aio_context;
+ BlockDriverState *image_bs = NULL;
+ Error *local_err = NULL;
+ bool ro;
+ int open_flags;
+ int ret;
+
+ blk = blk_by_name(device);
+ if (!blk) {
+ error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+ return;
+ }
+ bs = blk_bs(blk);
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ image_bs = bdrv_lookup_bs(NULL, image_node_name, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ goto out;
+ }
+
+ if (!image_bs) {
+ error_setg(errp, "image file not found");
+ goto out;
+ }
+
+ if (bdrv_find_base(image_bs) == image_bs) {
+ error_setg(errp, "not allowing backing file change on an image "
+ "without a backing file");
+ goto out;
+ }
+
+ /* even though we are not necessarily operating on bs, we need it to
+ * determine if block ops are currently prohibited on the chain */
+ if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_CHANGE, errp)) {
+ goto out;
+ }
+
+ /* final sanity check */
+ if (!bdrv_chain_contains(bs, image_bs)) {
+ error_setg(errp, "'%s' and image file are not in the same chain",
+ device);
+ goto out;
+ }
+
+ /* if not r/w, reopen to make r/w */
+ open_flags = image_bs->open_flags;
+ ro = bdrv_is_read_only(image_bs);
+
+ if (ro) {
+ bdrv_reopen(image_bs, open_flags | BDRV_O_RDWR, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ goto out;
+ }
+ }
+
+ ret = bdrv_change_backing_file(image_bs, backing_file,
+ image_bs->drv ? image_bs->drv->format_name : "");
+
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not change backing file to '%s'",
+ backing_file);
+ /* don't exit here, so we can try to restore open flags if
+ * appropriate */
+ }
+
+ if (ro) {
+ bdrv_reopen(image_bs, open_flags, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err); /* will preserve prior errp */
+ }
+ }
+
+out:
+ aio_context_release(aio_context);
+}
+
+void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
+{
+ QmpOutputVisitor *ov = qmp_output_visitor_new();
+ BlockBackend *blk;
+ QObject *obj;
+ QDict *qdict;
+ Error *local_err = NULL;
+
+ /* Require an ID in the top level */
+ if (!options->has_id) {
+ error_setg(errp, "Block device needs an ID");
+ goto fail;
+ }
+
+ /* TODO Sort it out in raw-posix and drive_new(): Reject aio=native with
+ * cache.direct=false instead of silently switching to aio=threads, except
+ * when called from drive_new().
+ *
+ * For now, simply forbidding the combination for all drivers will do. */
+ if (options->has_aio && options->aio == BLOCKDEV_AIO_OPTIONS_NATIVE) {
+ bool direct = options->has_cache &&
+ options->cache->has_direct &&
+ options->cache->direct;
+ if (!direct) {
+ error_setg(errp, "aio=native requires cache.direct=true");
+ goto fail;
+ }
+ }
+
+ visit_type_BlockdevOptions(qmp_output_get_visitor(ov),
+ &options, NULL, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ goto fail;
+ }
+
+ obj = qmp_output_get_qobject(ov);
+ qdict = qobject_to_qdict(obj);
+
+ qdict_flatten(qdict);
+
+ blk = blockdev_init(NULL, qdict, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ goto fail;
+ }
+
+ if (bdrv_key_required(blk_bs(blk))) {
+ blk_unref(blk);
+ error_setg(errp, "blockdev-add doesn't support encrypted devices");
+ goto fail;
+ }
+
+fail:
+ qmp_output_visitor_cleanup(ov);
+}
+
+BlockJobInfoList *qmp_query_block_jobs(Error **errp)
+{
+ BlockJobInfoList *head = NULL, **p_next = &head;
+ BlockDriverState *bs;
+
+ for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) {
+ AioContext *aio_context = bdrv_get_aio_context(bs);
+
+ aio_context_acquire(aio_context);
+
+ if (bs->job) {
+ BlockJobInfoList *elem = g_new0(BlockJobInfoList, 1);
+ elem->value = block_job_query(bs->job);
+ *p_next = elem;
+ p_next = &elem->next;
+ }
+
+ aio_context_release(aio_context);
+ }
+
+ return head;
+}
+
+/******************************************************************************/
+/* Transactions */
typedef struct BlkActionState BlkActionState;
@@ -2149,1294 +3439,7 @@ exit:
transaction_job_complete(bts);
}
-
-static void eject_device(BlockBackend *blk, int force, Error **errp)
-{
- BlockDriverState *bs = blk_bs(blk);
- AioContext *aio_context;
-
- aio_context = bdrv_get_aio_context(bs);
- aio_context_acquire(aio_context);
-
- if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) {
- goto out;
- }
- if (!blk_dev_has_removable_media(blk)) {
- error_setg(errp, "Device '%s' is not removable",
- bdrv_get_device_name(bs));
- goto out;
- }
-
- if (blk_dev_is_medium_locked(blk) && !blk_dev_is_tray_open(blk)) {
- blk_dev_eject_request(blk, force);
- if (!force) {
- error_setg(errp, "Device '%s' is locked",
- bdrv_get_device_name(bs));
- goto out;
- }
- }
-
- bdrv_close(bs);
-
-out:
- aio_context_release(aio_context);
-}
-
-void qmp_eject(const char *device, bool has_force, bool force, Error **errp)
-{
- BlockBackend *blk;
-
- blk = blk_by_name(device);
- if (!blk) {
- error_set(errp, QERR_DEVICE_NOT_FOUND, device);
- return;
- }
-
- eject_device(blk, force, errp);
-}
-
-void qmp_block_passwd(bool has_device, const char *device,
- bool has_node_name, const char *node_name,
- const char *password, Error **errp)
-{
- Error *local_err = NULL;
- BlockDriverState *bs;
- AioContext *aio_context;
-
- bs = bdrv_lookup_bs(has_device ? device : NULL,
- has_node_name ? node_name : NULL,
- &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- return;
- }
-
- aio_context = bdrv_get_aio_context(bs);
- aio_context_acquire(aio_context);
-
- bdrv_add_key(bs, password, errp);
-
- aio_context_release(aio_context);
-}
-
-/* Assumes AioContext is held */
-static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
- int bdrv_flags, BlockDriver *drv,
- const char *password, Error **errp)
-{
- Error *local_err = NULL;
- int ret;
-
- ret = bdrv_open(&bs, filename, NULL, NULL, bdrv_flags, drv, &local_err);
- if (ret < 0) {
- error_propagate(errp, local_err);
- return;
- }
-
- bdrv_add_key(bs, password, errp);
-}
-
-void qmp_change_blockdev(const char *device, const char *filename,
- const char *format, Error **errp)
-{
- BlockBackend *blk;
- BlockDriverState *bs;
- AioContext *aio_context;
- BlockDriver *drv = NULL;
- int bdrv_flags;
- Error *err = NULL;
-
- blk = blk_by_name(device);
- if (!blk) {
- error_set(errp, QERR_DEVICE_NOT_FOUND, device);
- return;
- }
- bs = blk_bs(blk);
-
- aio_context = bdrv_get_aio_context(bs);
- aio_context_acquire(aio_context);
-
- if (format) {
- drv = bdrv_find_whitelisted_format(format, bs->read_only);
- if (!drv) {
- error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
- goto out;
- }
- }
-
- eject_device(blk, 0, &err);
- if (err) {
- error_propagate(errp, err);
- goto out;
- }
-
- bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
- bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
-
- qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, errp);
-
-out:
- aio_context_release(aio_context);
-}
-
-/* throttling disk I/O limits */
-void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
- int64_t bps_wr,
- int64_t iops,
- int64_t iops_rd,
- int64_t iops_wr,
- bool has_bps_max,
- int64_t bps_max,
- bool has_bps_rd_max,
- int64_t bps_rd_max,
- bool has_bps_wr_max,
- int64_t bps_wr_max,
- bool has_iops_max,
- int64_t iops_max,
- bool has_iops_rd_max,
- int64_t iops_rd_max,
- bool has_iops_wr_max,
- int64_t iops_wr_max,
- bool has_iops_size,
- int64_t iops_size, Error **errp)
-{
- ThrottleConfig cfg;
- BlockDriverState *bs;
- BlockBackend *blk;
- AioContext *aio_context;
-
- blk = blk_by_name(device);
- if (!blk) {
- error_set(errp, QERR_DEVICE_NOT_FOUND, device);
- return;
- }
- bs = blk_bs(blk);
-
- memset(&cfg, 0, sizeof(cfg));
- cfg.buckets[THROTTLE_BPS_TOTAL].avg = bps;
- cfg.buckets[THROTTLE_BPS_READ].avg = bps_rd;
- cfg.buckets[THROTTLE_BPS_WRITE].avg = bps_wr;
-
- cfg.buckets[THROTTLE_OPS_TOTAL].avg = iops;
- cfg.buckets[THROTTLE_OPS_READ].avg = iops_rd;
- cfg.buckets[THROTTLE_OPS_WRITE].avg = iops_wr;
-
- if (has_bps_max) {
- cfg.buckets[THROTTLE_BPS_TOTAL].max = bps_max;
- }
- if (has_bps_rd_max) {
- cfg.buckets[THROTTLE_BPS_READ].max = bps_rd_max;
- }
- if (has_bps_wr_max) {
- cfg.buckets[THROTTLE_BPS_WRITE].max = bps_wr_max;
- }
- if (has_iops_max) {
- cfg.buckets[THROTTLE_OPS_TOTAL].max = iops_max;
- }
- if (has_iops_rd_max) {
- cfg.buckets[THROTTLE_OPS_READ].max = iops_rd_max;
- }
- if (has_iops_wr_max) {
- cfg.buckets[THROTTLE_OPS_WRITE].max = iops_wr_max;
- }
-
- if (has_iops_size) {
- cfg.op_size = iops_size;
- }
-
- if (!check_throttle_config(&cfg, errp)) {
- return;
- }
-
- aio_context = bdrv_get_aio_context(bs);
- aio_context_acquire(aio_context);
-
- if (!bs->io_limits_enabled && throttle_enabled(&cfg)) {
- bdrv_io_limits_enable(bs);
- } else if (bs->io_limits_enabled && !throttle_enabled(&cfg)) {
- bdrv_io_limits_disable(bs);
- }
-
- if (bs->io_limits_enabled) {
- bdrv_set_io_limits(bs, &cfg);
- }
-
- aio_context_release(aio_context);
-}
-
-void qmp_block_dirty_bitmap_add(const char *node, const char *name,
- bool has_granularity, uint32_t granularity,
- Error **errp)
-{
- AioContext *aio_context;
- BlockDriverState *bs;
-
- if (!name || name[0] == '\0') {
- error_setg(errp, "Bitmap name cannot be empty");
- return;
- }
-
- bs = bdrv_lookup_bs(node, node, errp);
- if (!bs) {
- return;
- }
-
- aio_context = bdrv_get_aio_context(bs);
- aio_context_acquire(aio_context);
-
- if (has_granularity) {
- if (granularity < 512 || !is_power_of_2(granularity)) {
- error_setg(errp, "Granularity must be power of 2 "
- "and at least 512");
- goto out;
- }
- } else {
- /* Default to cluster size, if available: */
- granularity = bdrv_get_default_bitmap_granularity(bs);
- }
-
- bdrv_create_dirty_bitmap(bs, granularity, name, errp);
-
- out:
- aio_context_release(aio_context);
-}
-
-void qmp_block_dirty_bitmap_remove(const char *node, const char *name,
- Error **errp)
-{
- AioContext *aio_context;
- BlockDriverState *bs;
- BdrvDirtyBitmap *bitmap;
-
- bitmap = block_dirty_bitmap_lookup(node, name, &bs, &aio_context, errp);
- if (!bitmap || !bs) {
- return;
- }
-
- if (bdrv_dirty_bitmap_frozen(bitmap)) {
- error_setg(errp,
- "Bitmap '%s' is currently frozen and cannot be removed",
- name);
- goto out;
- }
- bdrv_dirty_bitmap_make_anon(bitmap);
- bdrv_release_dirty_bitmap(bs, bitmap);
-
- out:
- aio_context_release(aio_context);
-}
-
-/**
- * Completely clear a bitmap, for the purposes of synchronizing a bitmap
- * immediately after a full backup operation.
- */
-void qmp_block_dirty_bitmap_clear(const char *node, const char *name,
- Error **errp)
-{
- AioContext *aio_context;
- BdrvDirtyBitmap *bitmap;
- BlockDriverState *bs;
-
- bitmap = block_dirty_bitmap_lookup(node, name, &bs, &aio_context, errp);
- if (!bitmap || !bs) {
- return;
- }
-
- if (bdrv_dirty_bitmap_frozen(bitmap)) {
- error_setg(errp,
- "Bitmap '%s' is currently frozen and cannot be modified",
- name);
- goto out;
- } else if (!bdrv_dirty_bitmap_enabled(bitmap)) {
- error_setg(errp,
- "Bitmap '%s' is currently disabled and cannot be cleared",
- name);
- goto out;
- }
-
- bdrv_clear_dirty_bitmap(bitmap);
-
- out:
- aio_context_release(aio_context);
-}
-
-int hmp_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
-{
- const char *id = qdict_get_str(qdict, "id");
- BlockBackend *blk;
- BlockDriverState *bs;
- AioContext *aio_context;
- Error *local_err = NULL;
-
- blk = blk_by_name(id);
- if (!blk) {
- error_report("Device '%s' not found", id);
- return -1;
- }
- bs = blk_bs(blk);
-
- if (!blk_legacy_dinfo(blk)) {
- error_report("Deleting device added with blockdev-add"
- " is not supported");
- return -1;
- }
-
- aio_context = bdrv_get_aio_context(bs);
- aio_context_acquire(aio_context);
-
- if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, &local_err)) {
- error_report_err(local_err);
- aio_context_release(aio_context);
- return -1;
- }
-
- /* quiesce block driver; prevent further io */
- bdrv_drain_all();
- bdrv_flush(bs);
- bdrv_close(bs);
-
- /* if we have a device attached to this BlockDriverState
- * then we need to make the drive anonymous until the device
- * can be removed. If this is a drive with no device backing
- * then we can just get rid of the block driver state right here.
- */
- if (blk_get_attached_dev(blk)) {
- blk_hide_on_behalf_of_hmp_drive_del(blk);
- /* Further I/O must not pause the guest */
- bdrv_set_on_error(bs, BLOCKDEV_ON_ERROR_REPORT,
- BLOCKDEV_ON_ERROR_REPORT);
- } else {
- blk_unref(blk);
- }
-
- aio_context_release(aio_context);
- return 0;
-}
-
-void qmp_block_resize(bool has_device, const char *device,
- bool has_node_name, const char *node_name,
- int64_t size, Error **errp)
-{
- Error *local_err = NULL;
- BlockDriverState *bs;
- AioContext *aio_context;
- int ret;
-
- bs = bdrv_lookup_bs(has_device ? device : NULL,
- has_node_name ? node_name : NULL,
- &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- return;
- }
-
- aio_context = bdrv_get_aio_context(bs);
- aio_context_acquire(aio_context);
-
- if (!bdrv_is_first_non_filter(bs)) {
- error_set(errp, QERR_FEATURE_DISABLED, "resize");
- goto out;
- }
-
- if (size < 0) {
- error_set(errp, QERR_INVALID_PARAMETER_VALUE, "size", "a >0 size");
- goto out;
- }
-
- if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) {
- error_set(errp, QERR_DEVICE_IN_USE, device);
- goto out;
- }
-
- /* complete all in-flight operations before resizing the device */
- bdrv_drain_all();
-
- ret = bdrv_truncate(bs, size);
- switch (ret) {
- case 0:
- break;
- case -ENOMEDIUM:
- error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
- break;
- case -ENOTSUP:
- error_set(errp, QERR_UNSUPPORTED);
- break;
- case -EACCES:
- error_set(errp, QERR_DEVICE_IS_READ_ONLY, device);
- break;
- case -EBUSY:
- error_set(errp, QERR_DEVICE_IN_USE, device);
- break;
- default:
- error_setg_errno(errp, -ret, "Could not resize");
- break;
- }
-
-out:
- aio_context_release(aio_context);
-}
-
-static void block_job_cb(void *opaque, int ret)
-{
- /* Note that this function may be executed from another AioContext besides
- * the QEMU main loop. If you need to access anything that assumes the
- * QEMU global mutex, use a BH or introduce a mutex.
- */
-
- BlockDriverState *bs = opaque;
- const char *msg = NULL;
-
- trace_block_job_cb(bs, bs->job, ret);
-
- assert(bs->job);
-
- if (ret < 0) {
- msg = strerror(-ret);
- }
-
- if (block_job_is_cancelled(bs->job)) {
- block_job_event_cancelled(bs->job);
- } else {
- block_job_event_completed(bs->job, msg);
- }
-
- bdrv_put_ref_bh_schedule(bs);
-}
-
-void qmp_block_stream(const char *device,
- bool has_base, const char *base,
- bool has_backing_file, const char *backing_file,
- bool has_speed, int64_t speed,
- bool has_on_error, BlockdevOnError on_error,
- Error **errp)
-{
- BlockBackend *blk;
- BlockDriverState *bs;
- BlockDriverState *base_bs = NULL;
- AioContext *aio_context;
- Error *local_err = NULL;
- const char *base_name = NULL;
-
- if (!has_on_error) {
- on_error = BLOCKDEV_ON_ERROR_REPORT;
- }
-
- blk = blk_by_name(device);
- if (!blk) {
- error_set(errp, QERR_DEVICE_NOT_FOUND, device);
- return;
- }
- bs = blk_bs(blk);
-
- aio_context = bdrv_get_aio_context(bs);
- aio_context_acquire(aio_context);
-
- if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_STREAM, errp)) {
- goto out;
- }
-
- if (has_base) {
- base_bs = bdrv_find_backing_image(bs, base);
- if (base_bs == NULL) {
- error_set(errp, QERR_BASE_NOT_FOUND, base);
- goto out;
- }
- assert(bdrv_get_aio_context(base_bs) == aio_context);
- base_name = base;
- }
-
- /* if we are streaming the entire chain, the result will have no backing
- * file, and specifying one is therefore an error */
- if (base_bs == NULL && has_backing_file) {
- error_setg(errp, "backing file specified, but streaming the "
- "entire chain");
- goto out;
- }
-
- /* backing_file string overrides base bs filename */
- base_name = has_backing_file ? backing_file : base_name;
-
- stream_start(bs, base_bs, base_name, has_speed ? speed : 0,
- on_error, block_job_cb, bs, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- goto out;
- }
-
- trace_qmp_block_stream(bs, bs->job);
-
-out:
- aio_context_release(aio_context);
-}
-
-void qmp_block_commit(const char *device,
- bool has_base, const char *base,
- bool has_top, const char *top,
- bool has_backing_file, const char *backing_file,
- bool has_speed, int64_t speed,
- Error **errp)
-{
- BlockBackend *blk;
- BlockDriverState *bs;
- BlockDriverState *base_bs, *top_bs;
- AioContext *aio_context;
- Error *local_err = NULL;
- /* This will be part of the QMP command, if/when the
- * BlockdevOnError change for blkmirror makes it in
- */
- BlockdevOnError on_error = BLOCKDEV_ON_ERROR_REPORT;
-
- if (!has_speed) {
- speed = 0;
- }
-
- /* Important Note:
- * libvirt relies on the DeviceNotFound error class in order to probe for
- * live commit feature versions; for this to work, we must make sure to
- * perform the device lookup before any generic errors that may occur in a
- * scenario in which all optional arguments are omitted. */
- blk = blk_by_name(device);
- if (!blk) {
- error_set(errp, QERR_DEVICE_NOT_FOUND, device);
- return;
- }
- bs = blk_bs(blk);
-
- aio_context = bdrv_get_aio_context(bs);
- aio_context_acquire(aio_context);
-
- /* drain all i/o before commits */
- bdrv_drain_all();
-
- if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT_SOURCE, errp)) {
- goto out;
- }
-
- /* default top_bs is the active layer */
- top_bs = bs;
-
- if (has_top && top) {
- if (strcmp(bs->filename, top) != 0) {
- top_bs = bdrv_find_backing_image(bs, top);
- }
- }
-
- if (top_bs == NULL) {
- error_setg(errp, "Top image file %s not found", top ? top : "NULL");
- goto out;
- }
-
- assert(bdrv_get_aio_context(top_bs) == aio_context);
-
- if (has_base && base) {
- base_bs = bdrv_find_backing_image(top_bs, base);
- } else {
- base_bs = bdrv_find_base(top_bs);
- }
-
- if (base_bs == NULL) {
- error_set(errp, QERR_BASE_NOT_FOUND, base ? base : "NULL");
- goto out;
- }
-
- assert(bdrv_get_aio_context(base_bs) == aio_context);
-
- if (bdrv_op_is_blocked(base_bs, BLOCK_OP_TYPE_COMMIT_TARGET, errp)) {
- goto out;
- }
-
- /* Do not allow attempts to commit an image into itself */
- if (top_bs == base_bs) {
- error_setg(errp, "cannot commit an image into itself");
- goto out;
- }
-
- if (top_bs == bs) {
- if (has_backing_file) {
- error_setg(errp, "'backing-file' specified,"
- " but 'top' is the active layer");
- goto out;
- }
- commit_active_start(bs, base_bs, speed, on_error, block_job_cb,
- bs, &local_err);
- } else {
- commit_start(bs, base_bs, top_bs, speed, on_error, block_job_cb, bs,
- has_backing_file ? backing_file : NULL, &local_err);
- }
- if (local_err != NULL) {
- error_propagate(errp, local_err);
- goto out;
- }
-
-out:
- aio_context_release(aio_context);
-}
-
-void qmp_drive_backup(const char *device, const char *target,
- bool has_format, const char *format,
- enum MirrorSyncMode sync,
- bool has_mode, enum NewImageMode mode,
- bool has_speed, int64_t speed,
- bool has_bitmap, const char *bitmap,
- bool has_on_source_error, BlockdevOnError on_source_error,
- bool has_on_target_error, BlockdevOnError on_target_error,
- Error **errp)
-{
- BlockBackend *blk;
- BlockDriverState *bs;
- BlockDriverState *target_bs;
- BlockDriverState *source = NULL;
- BdrvDirtyBitmap *bmap = NULL;
- AioContext *aio_context;
- BlockDriver *drv = NULL;
- Error *local_err = NULL;
- int flags;
- int64_t size;
- int ret;
-
- if (!has_speed) {
- speed = 0;
- }
- if (!has_on_source_error) {
- on_source_error = BLOCKDEV_ON_ERROR_REPORT;
- }
- if (!has_on_target_error) {
- on_target_error = BLOCKDEV_ON_ERROR_REPORT;
- }
- if (!has_mode) {
- mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
- }
-
- blk = blk_by_name(device);
- if (!blk) {
- error_set(errp, QERR_DEVICE_NOT_FOUND, device);
- return;
- }
- bs = blk_bs(blk);
-
- aio_context = bdrv_get_aio_context(bs);
- aio_context_acquire(aio_context);
-
- /* Although backup_run has this check too, we need to use bs->drv below, so
- * do an early check redundantly. */
- if (!bdrv_is_inserted(bs)) {
- error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
- goto out;
- }
-
- if (!has_format) {
- format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
- }
- if (format) {
- drv = bdrv_find_format(format);
- if (!drv) {
- error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
- goto out;
- }
- }
-
- /* Early check to avoid creating target */
- if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
- goto out;
- }
-
- flags = bs->open_flags | BDRV_O_RDWR;
-
- /* See if we have a backing HD we can use to create our new image
- * on top of. */
- if (sync == MIRROR_SYNC_MODE_TOP) {
- source = bs->backing_hd;
- if (!source) {
- sync = MIRROR_SYNC_MODE_FULL;
- }
- }
- if (sync == MIRROR_SYNC_MODE_NONE) {
- source = bs;
- }
-
- size = bdrv_getlength(bs);
- if (size < 0) {
- error_setg_errno(errp, -size, "bdrv_getlength failed");
- goto out;
- }
-
- if (mode != NEW_IMAGE_MODE_EXISTING) {
- assert(format && drv);
- if (source) {
- bdrv_img_create(target, format, source->filename,
- source->drv->format_name, NULL,
- size, flags, &local_err, false);
- } else {
- bdrv_img_create(target, format, NULL, NULL, NULL,
- size, flags, &local_err, false);
- }
- }
-
- if (local_err) {
- error_propagate(errp, local_err);
- goto out;
- }
-
- target_bs = NULL;
- ret = bdrv_open(&target_bs, target, NULL, NULL, flags, drv, &local_err);
- if (ret < 0) {
- error_propagate(errp, local_err);
- goto out;
- }
-
- bdrv_set_aio_context(target_bs, aio_context);
-
- if (has_bitmap) {
- bmap = bdrv_find_dirty_bitmap(bs, bitmap);
- if (!bmap) {
- error_setg(errp, "Bitmap '%s' could not be found", bitmap);
- goto out;
- }
- }
-
- backup_start(bs, target_bs, speed, sync, bmap,
- on_source_error, on_target_error,
- block_job_cb, bs, &local_err);
- if (local_err != NULL) {
- bdrv_unref(target_bs);
- error_propagate(errp, local_err);
- goto out;
- }
-
-out:
- aio_context_release(aio_context);
-}
-
-BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp)
-{
- return bdrv_named_nodes_list();
-}
-
-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,
- Error **errp)
-{
- BlockBackend *blk;
- BlockDriverState *bs;
- BlockDriverState *target_bs;
- Error *local_err = NULL;
- AioContext *aio_context;
-
- if (!has_speed) {
- speed = 0;
- }
- if (!has_on_source_error) {
- on_source_error = BLOCKDEV_ON_ERROR_REPORT;
- }
- if (!has_on_target_error) {
- on_target_error = BLOCKDEV_ON_ERROR_REPORT;
- }
-
- blk = blk_by_name(device);
- if (!blk) {
- error_setg(errp, "Device '%s' not found", device);
- return;
- }
- bs = blk_bs(blk);
-
- aio_context = bdrv_get_aio_context(bs);
- aio_context_acquire(aio_context);
-
- blk = blk_by_name(target);
- if (!blk) {
- error_setg(errp, "Device '%s' not found", target);
- goto out;
- }
- target_bs = blk_bs(blk);
-
- bdrv_ref(target_bs);
- bdrv_set_aio_context(target_bs, aio_context);
- backup_start(bs, target_bs, speed, sync, NULL, on_source_error,
- on_target_error, block_job_cb, bs, &local_err);
- if (local_err != NULL) {
- bdrv_unref(target_bs);
- error_propagate(errp, local_err);
- }
-out:
- aio_context_release(aio_context);
-}
-
-#define DEFAULT_MIRROR_BUF_SIZE (10 << 20)
-
-void qmp_drive_mirror(const char *device, const char *target,
- bool has_format, const char *format,
- bool has_node_name, const char *node_name,
- bool has_replaces, const char *replaces,
- enum MirrorSyncMode sync,
- bool has_mode, enum NewImageMode mode,
- bool has_speed, int64_t speed,
- bool has_granularity, uint32_t granularity,
- bool has_buf_size, int64_t buf_size,
- bool has_on_source_error, BlockdevOnError on_source_error,
- bool has_on_target_error, BlockdevOnError on_target_error,
- Error **errp)
-{
- BlockBackend *blk;
- BlockDriverState *bs;
- BlockDriverState *source, *target_bs;
- AioContext *aio_context;
- BlockDriver *drv = NULL;
- Error *local_err = NULL;
- QDict *options = NULL;
- int flags;
- int64_t size;
- int ret;
-
- if (!has_speed) {
- speed = 0;
- }
- if (!has_on_source_error) {
- on_source_error = BLOCKDEV_ON_ERROR_REPORT;
- }
- if (!has_on_target_error) {
- on_target_error = BLOCKDEV_ON_ERROR_REPORT;
- }
- if (!has_mode) {
- mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
- }
- if (!has_granularity) {
- granularity = 0;
- }
- if (!has_buf_size) {
- buf_size = DEFAULT_MIRROR_BUF_SIZE;
- }
-
- if (granularity != 0 && (granularity < 512 || granularity > 1048576 * 64)) {
- error_set(errp, QERR_INVALID_PARAMETER_VALUE, "granularity",
- "a value in range [512B, 64MB]");
- return;
- }
- if (granularity & (granularity - 1)) {
- error_set(errp, QERR_INVALID_PARAMETER_VALUE, "granularity", "power of 2");
- return;
- }
-
- blk = blk_by_name(device);
- if (!blk) {
- error_set(errp, QERR_DEVICE_NOT_FOUND, device);
- return;
- }
- bs = blk_bs(blk);
-
- aio_context = bdrv_get_aio_context(bs);
- aio_context_acquire(aio_context);
-
- if (!bdrv_is_inserted(bs)) {
- error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
- goto out;
- }
-
- if (!has_format) {
- format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
- }
- if (format) {
- drv = bdrv_find_format(format);
- if (!drv) {
- error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
- goto out;
- }
- }
-
- if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR, errp)) {
- goto out;
- }
-
- flags = bs->open_flags | BDRV_O_RDWR;
- source = bs->backing_hd;
- if (!source && sync == MIRROR_SYNC_MODE_TOP) {
- sync = MIRROR_SYNC_MODE_FULL;
- }
- if (sync == MIRROR_SYNC_MODE_NONE) {
- source = bs;
- }
-
- size = bdrv_getlength(bs);
- if (size < 0) {
- error_setg_errno(errp, -size, "bdrv_getlength failed");
- goto out;
- }
-
- if (has_replaces) {
- BlockDriverState *to_replace_bs;
- AioContext *replace_aio_context;
- int64_t replace_size;
-
- if (!has_node_name) {
- error_setg(errp, "a node-name must be provided when replacing a"
- " named node of the graph");
- goto out;
- }
-
- to_replace_bs = check_to_replace_node(replaces, &local_err);
-
- if (!to_replace_bs) {
- error_propagate(errp, local_err);
- goto out;
- }
-
- replace_aio_context = bdrv_get_aio_context(to_replace_bs);
- aio_context_acquire(replace_aio_context);
- replace_size = bdrv_getlength(to_replace_bs);
- aio_context_release(replace_aio_context);
-
- if (size != replace_size) {
- error_setg(errp, "cannot replace image with a mirror image of "
- "different size");
- goto out;
- }
- }
-
- if ((sync == MIRROR_SYNC_MODE_FULL || !source)
- && mode != NEW_IMAGE_MODE_EXISTING)
- {
- /* create new image w/o backing file */
- assert(format && drv);
- bdrv_img_create(target, format,
- NULL, NULL, NULL, size, flags, &local_err, false);
- } else {
- switch (mode) {
- case NEW_IMAGE_MODE_EXISTING:
- break;
- case NEW_IMAGE_MODE_ABSOLUTE_PATHS:
- /* create new image with backing file */
- bdrv_img_create(target, format,
- source->filename,
- source->drv->format_name,
- NULL, size, flags, &local_err, false);
- break;
- default:
- abort();
- }
- }
-
- if (local_err) {
- error_propagate(errp, local_err);
- goto out;
- }
-
- if (has_node_name) {
- options = qdict_new();
- qdict_put(options, "node-name", qstring_from_str(node_name));
- }
-
- /* Mirroring takes care of copy-on-write using the source's backing
- * file.
- */
- target_bs = NULL;
- ret = bdrv_open(&target_bs, target, NULL, options,
- flags | BDRV_O_NO_BACKING, drv, &local_err);
- if (ret < 0) {
- error_propagate(errp, local_err);
- goto out;
- }
-
- bdrv_set_aio_context(target_bs, aio_context);
-
- /* pass the node name to replace to mirror start since it's loose coupling
- * and will allow to check whether the node still exist at mirror completion
- */
- mirror_start(bs, target_bs,
- has_replaces ? replaces : NULL,
- speed, granularity, buf_size, sync,
- on_source_error, on_target_error,
- block_job_cb, bs, &local_err);
- if (local_err != NULL) {
- bdrv_unref(target_bs);
- error_propagate(errp, local_err);
- goto out;
- }
-
-out:
- aio_context_release(aio_context);
-}
-
-/* Get the block job for a given device name and acquire its AioContext */
-static BlockJob *find_block_job(const char *device, AioContext **aio_context,
- Error **errp)
-{
- BlockBackend *blk;
- BlockDriverState *bs;
-
- blk = blk_by_name(device);
- if (!blk) {
- goto notfound;
- }
- bs = blk_bs(blk);
-
- *aio_context = bdrv_get_aio_context(bs);
- aio_context_acquire(*aio_context);
-
- if (!bs->job) {
- aio_context_release(*aio_context);
- goto notfound;
- }
-
- return bs->job;
-
-notfound:
- error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE,
- "No active block job on device '%s'", device);
- *aio_context = NULL;
- return NULL;
-}
-
-void qmp_block_job_set_speed(const char *device, int64_t speed, Error **errp)
-{
- AioContext *aio_context;
- BlockJob *job = find_block_job(device, &aio_context, errp);
-
- if (!job) {
- return;
- }
-
- block_job_set_speed(job, speed, errp);
- aio_context_release(aio_context);
-}
-
-void qmp_block_job_cancel(const char *device,
- bool has_force, bool force, Error **errp)
-{
- AioContext *aio_context;
- BlockJob *job = find_block_job(device, &aio_context, errp);
-
- if (!job) {
- return;
- }
-
- if (!has_force) {
- force = false;
- }
-
- if (job->paused && !force) {
- error_setg(errp, "The block job for device '%s' is currently paused",
- device);
- goto out;
- }
-
- trace_qmp_block_job_cancel(job);
- block_job_cancel(job);
-out:
- aio_context_release(aio_context);
-}
-
-void qmp_block_job_pause(const char *device, Error **errp)
-{
- AioContext *aio_context;
- BlockJob *job = find_block_job(device, &aio_context, errp);
-
- if (!job) {
- return;
- }
-
- trace_qmp_block_job_pause(job);
- block_job_pause(job);
- aio_context_release(aio_context);
-}
-
-void qmp_block_job_resume(const char *device, Error **errp)
-{
- AioContext *aio_context;
- BlockJob *job = find_block_job(device, &aio_context, errp);
-
- if (!job) {
- return;
- }
-
- trace_qmp_block_job_resume(job);
- block_job_resume(job);
- aio_context_release(aio_context);
-}
-
-void qmp_block_job_complete(const char *device, Error **errp)
-{
- AioContext *aio_context;
- BlockJob *job = find_block_job(device, &aio_context, errp);
-
- if (!job) {
- return;
- }
-
- trace_qmp_block_job_complete(job);
- block_job_complete(job, errp);
- aio_context_release(aio_context);
-}
-
-void qmp_change_backing_file(const char *device,
- const char *image_node_name,
- const char *backing_file,
- Error **errp)
-{
- BlockBackend *blk;
- BlockDriverState *bs = NULL;
- AioContext *aio_context;
- BlockDriverState *image_bs = NULL;
- Error *local_err = NULL;
- bool ro;
- int open_flags;
- int ret;
-
- blk = blk_by_name(device);
- if (!blk) {
- error_set(errp, QERR_DEVICE_NOT_FOUND, device);
- return;
- }
- bs = blk_bs(blk);
-
- aio_context = bdrv_get_aio_context(bs);
- aio_context_acquire(aio_context);
-
- image_bs = bdrv_lookup_bs(NULL, image_node_name, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- goto out;
- }
-
- if (!image_bs) {
- error_setg(errp, "image file not found");
- goto out;
- }
-
- if (bdrv_find_base(image_bs) == image_bs) {
- error_setg(errp, "not allowing backing file change on an image "
- "without a backing file");
- goto out;
- }
-
- /* even though we are not necessarily operating on bs, we need it to
- * determine if block ops are currently prohibited on the chain */
- if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_CHANGE, errp)) {
- goto out;
- }
-
- /* final sanity check */
- if (!bdrv_chain_contains(bs, image_bs)) {
- error_setg(errp, "'%s' and image file are not in the same chain",
- device);
- goto out;
- }
-
- /* if not r/w, reopen to make r/w */
- open_flags = image_bs->open_flags;
- ro = bdrv_is_read_only(image_bs);
-
- if (ro) {
- bdrv_reopen(image_bs, open_flags | BDRV_O_RDWR, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- goto out;
- }
- }
-
- ret = bdrv_change_backing_file(image_bs, backing_file,
- image_bs->drv ? image_bs->drv->format_name : "");
-
- if (ret < 0) {
- error_setg_errno(errp, -ret, "Could not change backing file to '%s'",
- backing_file);
- /* don't exit here, so we can try to restore open flags if
- * appropriate */
- }
-
- if (ro) {
- bdrv_reopen(image_bs, open_flags, &local_err);
- if (local_err) {
- error_propagate(errp, local_err); /* will preserve prior errp */
- }
- }
-
-out:
- aio_context_release(aio_context);
-}
-
-void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
-{
- QmpOutputVisitor *ov = qmp_output_visitor_new();
- BlockBackend *blk;
- QObject *obj;
- QDict *qdict;
- Error *local_err = NULL;
-
- /* Require an ID in the top level */
- if (!options->has_id) {
- error_setg(errp, "Block device needs an ID");
- goto fail;
- }
-
- /* TODO Sort it out in raw-posix and drive_new(): Reject aio=native with
- * cache.direct=false instead of silently switching to aio=threads, except
- * when called from drive_new().
- *
- * For now, simply forbidding the combination for all drivers will do. */
- if (options->has_aio && options->aio == BLOCKDEV_AIO_OPTIONS_NATIVE) {
- bool direct = options->has_cache &&
- options->cache->has_direct &&
- options->cache->direct;
- if (!direct) {
- error_setg(errp, "aio=native requires cache.direct=true");
- goto fail;
- }
- }
-
- visit_type_BlockdevOptions(qmp_output_get_visitor(ov),
- &options, NULL, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- goto fail;
- }
-
- obj = qmp_output_get_qobject(ov);
- qdict = qobject_to_qdict(obj);
-
- qdict_flatten(qdict);
-
- blk = blockdev_init(NULL, qdict, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- goto fail;
- }
-
- if (bdrv_key_required(blk_bs(blk))) {
- blk_unref(blk);
- error_setg(errp, "blockdev-add doesn't support encrypted devices");
- goto fail;
- }
-
-fail:
- qmp_output_visitor_cleanup(ov);
-}
-
-BlockJobInfoList *qmp_query_block_jobs(Error **errp)
-{
- BlockJobInfoList *head = NULL, **p_next = &head;
- BlockDriverState *bs;
-
- for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) {
- AioContext *aio_context = bdrv_get_aio_context(bs);
-
- aio_context_acquire(aio_context);
-
- if (bs->job) {
- BlockJobInfoList *elem = g_new0(BlockJobInfoList, 1);
- elem->value = block_job_query(bs->job);
- *p_next = elem;
- p_next = &elem->next;
- }
-
- aio_context_release(aio_context);
- }
-
- return head;
-}
+/******************************************************************************/
QemuOptsList qemu_common_drive_opts = {
.name = "drive",
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH v2 09/11] qmp: Add an implementation wrapper for qmp_drive_backup
2015-03-27 19:19 [Qemu-devel] [PATCH v2 00/11] block: incremental backup transactions John Snow
` (7 preceding siblings ...)
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 08/11] block: move transactions beneath qmp interfaces John Snow
@ 2015-03-27 19:20 ` John Snow
2015-04-17 16:12 ` Max Reitz
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 10/11] block: drive_backup transaction callback support John Snow
` (2 subsequent siblings)
11 siblings, 1 reply; 31+ messages in thread
From: John Snow @ 2015-03-27 19:20 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, mreitz, vsementsov, stefanha
We'd like to be able to specify the callback given to backup_start
manually in the case of transactions, so split apart qmp_drive_backup
into an implementation and a wrapper.
Switch drive_backup_prepare to use the new wrapper, but don't overload
the callback and closure yet.
Signed-off-by: John Snow <jsnow@redhat.com>
---
blockdev.c | 65 ++++++++++++++++++++++++++++++++++++++++++++------------------
1 file changed, 46 insertions(+), 19 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index fa954e9..16b2cf7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1847,15 +1847,18 @@ out:
aio_context_release(aio_context);
}
-void qmp_drive_backup(const char *device, const char *target,
- bool has_format, const char *format,
- enum MirrorSyncMode sync,
- bool has_mode, enum NewImageMode mode,
- bool has_speed, int64_t speed,
- bool has_bitmap, const char *bitmap,
- bool has_on_source_error, BlockdevOnError on_source_error,
- bool has_on_target_error, BlockdevOnError on_target_error,
- Error **errp)
+static void do_drive_backup(const char *device, const char *target,
+ bool has_format, const char *format,
+ enum MirrorSyncMode sync,
+ bool has_mode, enum NewImageMode mode,
+ bool has_speed, int64_t speed,
+ bool has_bitmap, const char *bitmap,
+ bool has_on_source_error,
+ BlockdevOnError on_source_error,
+ bool has_on_target_error,
+ BlockdevOnError on_target_error,
+ BlockCompletionFunc *cb, void *opaque,
+ Error **errp)
{
BlockBackend *blk;
BlockDriverState *bs;
@@ -1969,9 +1972,16 @@ void qmp_drive_backup(const char *device, const char *target,
}
}
+ /* If we are not supplied with callback override info, use our defaults */
+ if (cb == NULL) {
+ cb = block_job_cb;
+ }
+ if (opaque == NULL) {
+ opaque = bs;
+ }
backup_start(bs, target_bs, speed, sync, bmap,
on_source_error, on_target_error,
- block_job_cb, bs, &local_err);
+ cb, opaque, &local_err);
if (local_err != NULL) {
bdrv_unref(target_bs);
error_propagate(errp, local_err);
@@ -1982,6 +1992,22 @@ out:
aio_context_release(aio_context);
}
+void qmp_drive_backup(const char *device, const char *target,
+ bool has_format, const char *format,
+ enum MirrorSyncMode sync,
+ bool has_mode, enum NewImageMode mode,
+ bool has_speed, int64_t speed,
+ bool has_bitmap, const char *bitmap,
+ bool has_on_source_error, BlockdevOnError on_source_error,
+ bool has_on_target_error, BlockdevOnError on_target_error,
+ Error **errp)
+{
+ do_drive_backup(device, target, has_format, format, sync, has_mode, mode,
+ has_speed, speed, has_bitmap, bitmap, has_on_source_error,
+ on_source_error, has_on_target_error, on_target_error,
+ NULL, NULL, errp);
+}
+
BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp)
{
return bdrv_named_nodes_list();
@@ -3112,15 +3138,16 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
state->aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(state->aio_context);
- qmp_drive_backup(backup->device, backup->target,
- backup->has_format, backup->format,
- backup->sync,
- backup->has_mode, backup->mode,
- backup->has_speed, backup->speed,
- backup->has_bitmap, backup->bitmap,
- backup->has_on_source_error, backup->on_source_error,
- backup->has_on_target_error, backup->on_target_error,
- &local_err);
+ do_drive_backup(backup->device, backup->target,
+ backup->has_format, backup->format,
+ backup->sync,
+ backup->has_mode, backup->mode,
+ backup->has_speed, backup->speed,
+ backup->has_bitmap, backup->bitmap,
+ backup->has_on_source_error, backup->on_source_error,
+ backup->has_on_target_error, backup->on_target_error,
+ NULL, NULL,
+ &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH v2 10/11] block: drive_backup transaction callback support
2015-03-27 19:19 [Qemu-devel] [PATCH v2 00/11] block: incremental backup transactions John Snow
` (8 preceding siblings ...)
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 09/11] qmp: Add an implementation wrapper for qmp_drive_backup John Snow
@ 2015-03-27 19:20 ` John Snow
2015-04-17 16:55 ` Max Reitz
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 11/11] iotests: 124 - transactional failure test John Snow
2015-04-18 1:01 ` [Qemu-devel] [PATCH v2.5 00/10] block: incremental backup transactions John Snow
11 siblings, 1 reply; 31+ messages in thread
From: John Snow @ 2015-03-27 19:20 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, mreitz, vsementsov, stefanha
This patch actually implements the transactional callback system
for the drive_backup transaction.
(1) We manually pick up a reference to the bitmap if present to allow
its cleanup to be delayed until after all drive_backup jobs launched
by the transaction have fully completed.
(2) We create a functional closure that envelops the original drive_backup
callback, to be able to intercept the completion status and return code
for the job.
(3) We add the drive_backup_cb method for the drive_backup action, which
unpacks the completion information and invokes the final cleanup.
(4) backup_transaction_complete will perform the final cleanup on the
backup job.
(5) In the case of transaction cancellation, drive_backup_cb is still
responsible for cleaning up the mess we may have already made.
Signed-off-by: John Snow <jsnow@redhat.com>
---
block/backup.c | 9 ++++++++
blockdev.c | 52 ++++++++++++++++++++++++++++++++++++++++++++---
include/block/block_int.h | 8 ++++++++
3 files changed, 66 insertions(+), 3 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index 1fefca7..f526141 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -233,6 +233,15 @@ typedef struct {
int ret;
} BackupCompleteData;
+void backup_transaction_complete(BlockJob *job, int ret)
+{
+ BackupBlockJob *s = container_of(job, BackupBlockJob, common);
+
+ if (s->sync_bitmap) {
+ bdrv_frozen_bitmap_decref(job->bs, s->sync_bitmap, ret);
+ }
+}
+
static void backup_complete(BlockJob *job, void *opaque)
{
BackupBlockJob *s = container_of(job, BackupBlockJob, common);
diff --git a/blockdev.c b/blockdev.c
index 16b2cf7..f529ac2 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2749,7 +2749,6 @@ static void transaction_action_callback(void *opaque, int ret)
*
* @return The callback to be used instead of @callback.
*/
-__attribute__((__unused__))
static CallbackFn *new_action_cb_wrapper(BlkActionState *common,
void *opaque,
CallbackFn *callback,
@@ -2777,7 +2776,6 @@ static CallbackFn *new_action_cb_wrapper(BlkActionState *common,
/**
* Undo any actions performed by the above call.
*/
-__attribute__((__unused__))
static void cancel_action_cb_wrapper(BlkActionState *common)
{
/* Stage 0: Wrapper was never created: */
@@ -3123,6 +3121,9 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
BlockBackend *blk;
DriveBackup *backup;
Error *local_err = NULL;
+ CallbackFn *cb;
+ void *opaque;
+ BdrvDirtyBitmap *bmap = NULL;
assert(common->action->kind == TRANSACTION_ACTION_KIND_DRIVE_BACKUP);
backup = common->action->drive_backup;
@@ -3134,6 +3135,19 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
}
bs = blk_bs(blk);
+ /* BackupBlockJob is opaque to us, so look up the bitmap ourselves */
+ if (backup->has_bitmap) {
+ bmap = bdrv_find_dirty_bitmap(bs, backup->bitmap);
+ if (!bmap) {
+ error_setg(errp, "Bitmap '%s' could not be found", backup->bitmap);
+ return;
+ }
+ }
+
+ /* Create our transactional callback wrapper,
+ and register that we'd like to call .cb() later. */
+ cb = new_action_cb_wrapper(common, bs, block_job_cb, &opaque);
+
/* AioContext is released in .clean() */
state->aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(state->aio_context);
@@ -3146,7 +3160,7 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
backup->has_bitmap, backup->bitmap,
backup->has_on_source_error, backup->on_source_error,
backup->has_on_target_error, backup->on_target_error,
- NULL, NULL,
+ cb, opaque,
&local_err);
if (local_err) {
error_propagate(errp, local_err);
@@ -3155,6 +3169,12 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
state->bs = bs;
state->job = state->bs->job;
+ /* Keep the job alive until .cb(), too:
+ * References are only incremented after the job launches successfully. */
+ block_job_incref(state->job);
+ if (bmap) {
+ bdrv_dirty_bitmap_incref(bmap);
+ }
}
static void drive_backup_abort(BlkActionState *common)
@@ -3166,6 +3186,10 @@ static void drive_backup_abort(BlkActionState *common)
if (bs && bs->job && bs->job == state->job) {
block_job_cancel_sync(bs->job);
}
+
+ /* Undo any callback actions we may have done. Putting down references
+ * obtained during prepare() is handled by cb(). */
+ cancel_action_cb_wrapper(common);
}
static void drive_backup_clean(BlkActionState *common)
@@ -3177,6 +3201,27 @@ static void drive_backup_clean(BlkActionState *common)
}
}
+static void drive_backup_cb(BlkActionState *common)
+{
+ BlkActionCallbackData *cb_data = common->cb_data;
+ BlockDriverState *bs = cb_data->opaque;
+ DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
+
+ assert(state->bs == bs);
+ if (bs->job) {
+ assert(state->job == bs->job);
+ }
+
+ state->aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(state->aio_context);
+
+ /* Note: We also have the individual job's return code in cb_data->ret */
+ backup_transaction_complete(state->job, common->transaction->status);
+ block_job_decref(state->job);
+
+ aio_context_release(state->aio_context);
+}
+
typedef struct BlockdevBackupState {
BlkActionState common;
BlockDriverState *bs;
@@ -3364,6 +3409,7 @@ static const BlkActionOps actions[] = {
.prepare = drive_backup_prepare,
.abort = drive_backup_abort,
.clean = drive_backup_clean,
+ .cb = drive_backup_cb,
},
[TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP] = {
.instance_size = sizeof(BlockdevBackupState),
diff --git a/include/block/block_int.h b/include/block/block_int.h
index e0d5561..0d26713 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -619,6 +619,14 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
BlockCompletionFunc *cb, void *opaque,
Error **errp);
+/*
+ * backup_transaction_complete:
+ * @job: The BackupBlockJob that was associated with a transaction.
+ * @ret: The collective return code for the entire transaction.
+ * Expects zero for success, non-zero for an error otherwise.
+ */
+void backup_transaction_complete(BlockJob *job, int ret);
+
void blk_dev_change_media_cb(BlockBackend *blk, bool load);
bool blk_dev_has_removable_media(BlockBackend *blk);
void blk_dev_eject_request(BlockBackend *blk, bool force);
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH v2 11/11] iotests: 124 - transactional failure test
2015-03-27 19:19 [Qemu-devel] [PATCH v2 00/11] block: incremental backup transactions John Snow
` (9 preceding siblings ...)
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 10/11] block: drive_backup transaction callback support John Snow
@ 2015-03-27 19:20 ` John Snow
2015-04-17 17:04 ` Max Reitz
2015-04-18 1:01 ` [Qemu-devel] [PATCH v2.5 00/10] block: incremental backup transactions John Snow
11 siblings, 1 reply; 31+ messages in thread
From: John Snow @ 2015-03-27 19:20 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, mreitz, vsementsov, stefanha
Use a transaction to request an incremental backup across two drives.
Coerce one of the jobs to fail, and then re-run the transaction.
Verify that no bitmap data was lost due to the partial transaction
failure.
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/124 | 119 +++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/124.out | 4 +-
2 files changed, 121 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index 31946f9..ad82076 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -332,6 +332,125 @@ class TestIncrementalBackup(iotests.QMPTestCase):
self.create_incremental()
+ def test_transaction_failure(self):
+ '''Test: Verify backups made from a transaction that partially fails.
+
+ Add a second drive with its own unique pattern, and add a bitmap to each
+ drive. Use blkdebug to interfere with the backup on just one drive and
+ attempt to create a coherent incremental backup across both drives.
+
+ verify a failure in one but not both, then delete the failed stubs and
+ re-run the same transaction.
+
+ verify that both incrementals are created successfully.
+ '''
+
+ # Create a second drive, with pattern:
+ drive1 = self.add_node('drive1')
+ self.img_create(drive1['file'], drive1['fmt'])
+ io_write_patterns(drive1['file'], (('0x14', 0, 512),
+ ('0x5d', '1M', '32k'),
+ ('0xcd', '32M', '124k')))
+
+ # Create a blkdebug interface to this img as 'drive1'
+ result = self.vm.qmp('blockdev-add', options={
+ 'id': drive1['id'],
+ 'driver': drive1['fmt'],
+ 'file': {
+ 'driver': 'blkdebug',
+ 'image': {
+ 'driver': 'file',
+ 'filename': drive1['file']
+ },
+ 'set-state': [{
+ 'event': 'flush_to_disk',
+ 'state': 1,
+ 'new_state': 2
+ }],
+ 'inject-error': [{
+ 'event': 'read_aio',
+ 'errno': 5,
+ 'state': 2,
+ 'immediately': False,
+ 'once': True
+ }],
+ }
+ })
+ self.assert_qmp(result, 'return', {})
+
+ # Create bitmaps and full backups for both drives
+ drive0 = self.drives[0]
+ dr0bm0 = self.add_bitmap('bitmap0', drive0)
+ dr1bm0 = self.add_bitmap('bitmap0', drive1)
+ self.create_full_backup(drive0)
+ self.create_full_backup(drive1)
+ self.assert_no_active_block_jobs()
+ self.assertFalse(self.vm.get_qmp_events(wait=False))
+
+ # Emulate some writes
+ self.hmp_io_writes(drive0['id'], (('0xab', 0, 512),
+ ('0xfe', '16M', '256k'),
+ ('0x64', '32736k', '64k')))
+ self.hmp_io_writes(drive1['id'], (('0xba', 0, 512),
+ ('0xef', '16M', '256k'),
+ ('0x46', '32736k', '64k')))
+
+ # Create incremental backup targets
+ target0 = self.prepare_backup(dr0bm0)
+ target1 = self.prepare_backup(dr1bm0)
+
+ # Ask for a new incremental backup per-each drive,
+ # expecting drive1's backup to fail:
+ transaction = [
+ {
+ 'type': 'drive-backup',
+ 'data': { 'device': drive0['id'],
+ 'sync': 'dirty-bitmap',
+ 'format': drive0['fmt'],
+ 'target': target0,
+ 'mode': 'existing',
+ 'bitmap': dr0bm0.name },
+ },
+ {
+ 'type': 'drive-backup',
+ 'data': { 'device': drive1['id'],
+ 'sync': 'dirty-bitmap',
+ 'format': drive1['fmt'],
+ 'target': target1,
+ 'mode': 'existing',
+ 'bitmap': dr1bm0.name }
+ }
+ ]
+ result = self.vm.qmp('transaction', actions=transaction)
+ self.assert_qmp(result, 'return', {})
+
+ # Observe that drive0's backup completes, but drive1's does not.
+ # Consume drive1's error and ensure all pending actions are completed.
+ self.wait_incremental(dr0bm0, validate=True)
+ self.wait_incremental(dr1bm0, validate=False)
+ error = self.vm.event_wait('BLOCK_JOB_ERROR')
+ self.assert_qmp(error, 'data', {'device': drive1['id'],
+ 'action': 'report',
+ 'operation': 'read'})
+ self.assertFalse(self.vm.get_qmp_events(wait=False))
+ self.assert_no_active_block_jobs()
+
+ # Delete drive0's (successful) backup and create two new empty
+ # targets to re-run the transaction.
+ dr0bm0.del_target()
+ target0 = self.prepare_backup(dr0bm0)
+ target1 = self.prepare_backup(dr1bm0)
+
+ # Re-run the exact same transaction.
+ result = self.vm.qmp('transaction', actions=transaction)
+ self.assert_qmp(result, 'return', {})
+ # Both should complete successfully this time.
+ self.wait_incremental(dr0bm0)
+ self.wait_incremental(dr1bm0)
+ self.assertFalse(self.vm.get_qmp_events(wait=False))
+ self.assert_no_active_block_jobs()
+
+
def test_sync_dirty_bitmap_missing(self):
self.assert_no_active_block_jobs()
self.files.append(self.err_img)
diff --git a/tests/qemu-iotests/124.out b/tests/qemu-iotests/124.out
index 914e373..3f8a935 100644
--- a/tests/qemu-iotests/124.out
+++ b/tests/qemu-iotests/124.out
@@ -1,5 +1,5 @@
-.....
+......
----------------------------------------------------------------------
-Ran 5 tests
+Ran 6 tests
OK
--
2.1.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 01/11] qapi: Add transaction support to block-dirty-bitmap operations
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 01/11] qapi: Add transaction support to block-dirty-bitmap operations John Snow
@ 2015-04-17 14:41 ` Max Reitz
2015-04-17 14:50 ` Eric Blake
1 sibling, 0 replies; 31+ messages in thread
From: Max Reitz @ 2015-04-17 14:41 UTC (permalink / raw)
To: John Snow, qemu-block; +Cc: kwolf, famz, qemu-devel, vsementsov, stefanha
On 27.03.2015 20:19, John Snow wrote:
> This adds two qmp commands to transactions.
>
> block-dirty-bitmap-add allows you to create a bitmap simultaneously
> alongside a new full backup to accomplish a clean synchronization
> point.
>
> block-dirty-bitmap-clear allows you to reset a bitmap back to as-if
> it were new, which can also be used alongside a full backup to
> accomplish a clean synchronization point.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> blockdev.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> qapi-schema.json | 6 +++-
> 2 files changed, 105 insertions(+), 1 deletion(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 02/11] iotests: add transactional incremental backup test
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 02/11] iotests: add transactional incremental backup test John Snow
@ 2015-04-17 14:42 ` Max Reitz
0 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2015-04-17 14:42 UTC (permalink / raw)
To: John Snow, qemu-block; +Cc: kwolf, famz, qemu-devel, vsementsov, stefanha
On 27.03.2015 20:19, John Snow wrote:
> Test simple usage cases for using transactions to create
> and synchronize incremental backups.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> tests/qemu-iotests/124 | 51 ++++++++++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/124.out | 4 ++--
> 2 files changed, 53 insertions(+), 2 deletions(-)
Saying the obvious: This patch will need fixup for v5 of the
transaction-less series.
Max
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 01/11] qapi: Add transaction support to block-dirty-bitmap operations
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 01/11] qapi: Add transaction support to block-dirty-bitmap operations John Snow
2015-04-17 14:41 ` Max Reitz
@ 2015-04-17 14:50 ` Eric Blake
1 sibling, 0 replies; 31+ messages in thread
From: Eric Blake @ 2015-04-17 14:50 UTC (permalink / raw)
To: John Snow, qemu-block
Cc: kwolf, famz, qemu-devel, mreitz, vsementsov, stefanha
[-- Attachment #1: Type: text/plain, Size: 864 bytes --]
On 03/27/2015 01:19 PM, John Snow wrote:
> This adds two qmp commands to transactions.
>
> block-dirty-bitmap-add allows you to create a bitmap simultaneously
> alongside a new full backup to accomplish a clean synchronization
> point.
>
> block-dirty-bitmap-clear allows you to reset a bitmap back to as-if
> it were new, which can also be used alongside a full backup to
> accomplish a clean synchronization point.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> blockdev.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> qapi-schema.json | 6 +++-
> 2 files changed, 105 insertions(+), 1 deletion(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 03/11] block: rename BlkTransactionState and BdrvActionOps
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 03/11] block: rename BlkTransactionState and BdrvActionOps John Snow
@ 2015-04-17 14:51 ` Max Reitz
0 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2015-04-17 14:51 UTC (permalink / raw)
To: John Snow, qemu-block; +Cc: kwolf, famz, qemu-devel, vsementsov, stefanha
On 27.03.2015 20:19, John Snow wrote:
> 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 <jsnow@redhat.com>
> ---
> blockdev.c | 114 ++++++++++++++++++++++++++++++++++---------------------------
> 1 file changed, 64 insertions(+), 50 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 04/11] block: re-add BlkTransactionState
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 04/11] block: re-add BlkTransactionState John Snow
@ 2015-04-17 15:11 ` Max Reitz
0 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2015-04-17 15:11 UTC (permalink / raw)
To: John Snow, qemu-block; +Cc: kwolf, famz, qemu-devel, vsementsov, stefanha
On 27.03.2015 20:19, John Snow wrote:
> Now that the structure formerly known as BlkTransactionState has been
> renamed to something sensible (BlkActionState), re-introduce an actual
> BlkTransactionState that actually manages state for the entire Transaction.
>
> In the process, convert the old QSIMPLEQ list of actions into a QTAILQ,
> to let us more efficiently delete items in arbitrary order, which will
> be more important in the future when some actions will expire at the end
> of the transaction, but others may persist until all callbacks triggered
> by the transaction are recollected.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> blockdev.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 59 insertions(+), 7 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 05/11] block: add transactional callbacks feature
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 05/11] block: add transactional callbacks feature John Snow
@ 2015-04-17 15:41 ` Max Reitz
2015-04-17 21:55 ` John Snow
0 siblings, 1 reply; 31+ messages in thread
From: Max Reitz @ 2015-04-17 15:41 UTC (permalink / raw)
To: John Snow, qemu-block; +Cc: kwolf, famz, qemu-devel, vsementsov, stefanha
On 27.03.2015 20:19, John Snow wrote:
> The goal here is to add a new method to transactions that allows
> developers to specify a callback that will get invoked only once
> all jobs spawned by a transaction are completed, allowing developers
> the chance to perform actions conditionally pending complete success,
> partial failure, or complete failure.
>
> In order to register the new callback to be invoked, a user must request
> a callback pointer and closure by calling new_action_cb_wrapper, which
> creates a wrapper around an opaque pointer and callback that would have
> originally been passed to e.g. backup_start().
>
> The function will return a function pointer and a new opaque pointer to
> be passed instead. The transaction system will effectively intercept the
> original callbacks and perform book-keeping on the transaction after it
> has delivered the original enveloped callback.
>
> This means that Transaction Action callback methods will be called after
> all callbacks triggered by all Actions in the Transactional group have
> been received.
>
> This feature has no knowledge of any jobs spawned by Actions that do not
> inform the system via new_action_cb_wrapper().
>
> For an example of how to use the feature, please skip ahead to:
> 'block: drive_backup transaction callback support' which serves as an example
> for how to hook up a post-transaction callback to the Drive Backup action.
>
>
> Note 1: Defining a callback method alone is not sufficient to have the new
> method invoked. You must call new_action_cb_wrapper() AND ensure the
> callback it returns is the one used as the callback for the job
> launched by the action.
>
> Note 2: You can use this feature for any system that registers completions of
> an asynchronous task via a callback of the form
> (void *opaque, int ret), not just block job callbacks.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> blockdev.c | 191 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 187 insertions(+), 4 deletions(-)
I like it much better than v1! :-)
Some minor comments below. (But just as in v1, I need to look into the
following patches to know exactly how some of the functions introduced
here are used)
> diff --git a/blockdev.c b/blockdev.c
> index f806d40..d404251 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1239,6 +1239,8 @@ typedef struct BlkActionState BlkActionState;
> * @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.
> + * @cb: Executed after all jobs launched by actions in the transaction finish,
> + * but only if requested by new_action_cb_wrapper() prior to clean().
> *
> * 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.
> @@ -1249,6 +1251,7 @@ typedef struct BlkActionOps {
> void (*commit)(BlkActionState *common);
> void (*abort)(BlkActionState *common);
> void (*clean)(BlkActionState *common);
> + void (*cb)(BlkActionState *common);
> } BlkActionOps;
>
> /**
> @@ -1257,19 +1260,46 @@ typedef struct BlkActionOps {
> * by a transaction group.
> *
> * @jobs: A reference count that tracks how many jobs still need to complete.
> + * @status: A cumulative return code for all actions that have reported
> + * a return code via callback in the transaction.
> * @actions: A list of all Actions in the Transaction.
> + * However, once the transaction has completed, it will be only a list
> + * of transactions that have registered a post-transaction callback.
> */
> typedef struct BlkTransactionState {
> int jobs;
> + int status;
> QTAILQ_HEAD(actions, BlkActionState) actions;
> } BlkTransactionState;
>
> +typedef void (CallbackFn)(void *opaque, int ret);
> +
> +/**
> + * BlkActionCallbackData:
> + * Necessary state for intercepting and
> + * re-delivering a callback triggered by an Action.
> + *
> + * @opaque: The data to be given to the encapsulated callback when
> + * a job launched by an Action completes.
> + * @ret: The status code that was delivered to the encapsulated callback.
> + * @callback: The encapsulated callback to invoke upon completion of
> + * the Job launched by the Action.
> + */
> +typedef struct BlkActionCallbackData {
> + void *opaque;
> + int ret;
> + CallbackFn *callback;
> +} BlkActionCallbackData;
> +
> /**
> * 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.
> + * @transaction: A pointer back to the Transaction this Action belongs to.
> + * @cb_data: Information on this Action's encapsulated callback, if any.
> + * @refcount: reference count, allowing access to this state beyond clean().
> * @entry: List membership for all Actions in this Transaction.
> *
> * This structure must be arranged as first member in a subclassed type,
> @@ -1279,6 +1309,9 @@ typedef struct BlkTransactionState {
> struct BlkActionState {
> TransactionAction *action;
> const BlkActionOps *ops;
> + BlkTransactionState *transaction;
> + BlkActionCallbackData *cb_data;
> + int refcount;
> QTAILQ_ENTRY(BlkActionState) entry;
> };
>
> @@ -1294,6 +1327,26 @@ static BlkTransactionState *new_blk_transaction_state(void)
> return bts;
> }
>
> +static BlkActionState *put_blk_action_state(BlkActionState *state)
> +{
> + BlkActionState *bas = state;
I don't think this variable needs to be initialized.
> +
> + state->refcount--;
> + if (state->refcount == 0) {
> +
Superfluous empty line.
> + QTAILQ_FOREACH(bas, &state->transaction->actions, entry) {
> + if (bas == state) {
> + QTAILQ_REMOVE(&state->transaction->actions, bas, entry);
> + break;
> + }
> + }
Wouldn't QTAILQ_REMOVE(&state->transaction->actions, state, entry); do
exactly the same thing as this loop? There'd be a difference only if
"state" is not element of the list of actions, but think that should be
impossible.
Max
> + g_free(state->cb_data);
> + g_free(state);
> + return NULL;
> + }
> + return state;
> +}
> +
> static void destroy_blk_transaction_state(BlkTransactionState *bts)
> {
> BlkActionState *bas, *bas_next;
> @@ -1301,23 +1354,151 @@ static void destroy_blk_transaction_state(BlkTransactionState *bts)
> /* The list should in normal cases be empty,
> * but in case someone really just wants to kibosh the whole deal: */
> QTAILQ_FOREACH_SAFE(bas, &bts->actions, entry, bas_next) {
> - QTAILQ_REMOVE(&bts->actions, bas, entry);
> - g_free(bas);
> + put_blk_action_state(bas);
> }
>
> g_free(bts);
> }
>
> +static void transaction_run_cb_action_ops(BlkTransactionState *bts)
> +{
> + BlkActionState *bas, *bas_next;
> +
> + QTAILQ_FOREACH_SAFE(bas, &bts->actions, entry, bas_next) {
> + if (bas->ops->cb) {
> + bas->ops->cb(bas);
> + }
> +
> + g_free(bas->cb_data);
> + bas->cb_data = NULL;
> + put_blk_action_state(bas);
> + }
> +}
> +
> static BlkTransactionState *transaction_job_complete(BlkTransactionState *bts)
> {
> bts->jobs--;
> if (bts->jobs == 0) {
> + transaction_run_cb_action_ops(bts);
> destroy_blk_transaction_state(bts);
> return NULL;
> }
> return bts;
> }
>
> +static void transaction_job_cancel(BlkActionState *bas)
> +{
> + assert(bas->transaction->jobs > 1);
> + transaction_job_complete(bas->transaction);
> +}
> +
> +/**
> + * Intercept a callback that was issued due to a transactional action.
> + */
> +static void transaction_action_callback(void *opaque, int ret)
> +{
> + BlkActionState *common = opaque;
> + BlkActionCallbackData *cb_data = common->cb_data;
> + CallbackFn *callback = cb_data->callback;
> +
> + /* Prepare data for ops->cb() */
> + cb_data->callback = NULL;
> + cb_data->ret = ret;
> +
> + /* Cumulative return code will track the first error discovered */
> + if (!common->transaction->status) {
> + common->transaction->status = ret;
> + }
> +
> + /* Deliver the intercepted callback prior to marking it as completed */
> + callback(cb_data->opaque, cb_data->ret);
> + transaction_job_complete(common->transaction);
> +}
> +
> +/**
> + * Create a new transactional callback wrapper.
> + *
> + * Given a callback and a closure, generate a new
> + * callback and closure that will invoke the
> + * given callback with the given closure.
> + *
> + * After all wrappers in the transactional group have
> + * been processed, each action's .cb() method will be
> + * invoked.
> + *
> + * @common The transaction action state to set a callback for.
> + * @opaque A closure intended for the encapsulated callback.
> + * @callback The callback we are encapsulating.
> + * @new_opaque[out]: The closure to be used instead of @opaque.
> + *
> + * @return The callback to be used instead of @callback.
> + */
> +__attribute__((__unused__))
> +static CallbackFn *new_action_cb_wrapper(BlkActionState *common,
> + void *opaque,
> + CallbackFn *callback,
> + void **new_opaque)
> +{
> + BlkActionCallbackData *cb_data = g_new0(BlkActionCallbackData, 1);
> + assert(new_opaque);
> +
> + /* Stash the original callback information */
> + cb_data->opaque = opaque;
> + cb_data->callback = callback;
> + common->cb_data = cb_data;
> +
> + /* The BAS itself will serve as our new closure */
> + *new_opaque = common;
> + common->refcount++;
> +
> + /* Inform the transaction to expect one more return */
> + common->transaction->jobs++;
> +
> + /* Lastly, the actual callback function to handle the interception. */
> + return transaction_action_callback;
> +}
> +
> +/**
> + * Undo any actions performed by the above call.
> + */
> +__attribute__((__unused__))
> +static void cancel_action_cb_wrapper(BlkActionState *common)
> +{
> + /* Stage 0: Wrapper was never created: */
> + if (common->cb_data == NULL) {
> + assert(common->refcount == 1);
> + return;
> + }
> +
> + /* Stage 1: Callback was created, but the job never launched.
> + * (We assume that the job cannot possibly be running, because
> + * we assume it has been synchronously canceled if it was.)
> + *
> + * We also assume that any cleanup normally handled by cb()
> + * is not needed, because it should have been prepared after
> + * the job launched.
> + */
> + if (common->cb_data && common->cb_data->callback) {
> + transaction_job_cancel(common);
> + goto cleanup;
> + }
> +
> + /* Stage 2: Job already completed, or was canceled.
> + *
> + * Force an error in the callback data and just invoke the completion
> + * handler to perform appropriate cleanup for us.
> + */
> + if (common->cb_data && !common->cb_data->callback) {
> + common->cb_data->ret = -ECANCELED;
> + common->ops->cb(common);
> + }
> +
> + cleanup:
> + g_free(common->cb_data);
> + common->cb_data = NULL;
> + put_blk_action_state(common);
> +}
> +
> /* internal snapshot private data */
> typedef struct InternalSnapshotState {
> BlkActionState common;
> @@ -1927,8 +2108,10 @@ void qmp_transaction(TransactionActionList *dev_list, Error **errp)
> assert(ops->instance_size > 0);
>
> state = g_malloc0(ops->instance_size);
> + state->refcount = 1;
> state->ops = ops;
> state->action = dev_info;
> + state->transaction = bts;
> QTAILQ_INSERT_TAIL(&bts->actions, state, entry);
>
> state->ops->prepare(state, &local_err);
> @@ -1959,10 +2142,10 @@ exit:
> if (state->ops->clean) {
> state->ops->clean(state);
> }
> - QTAILQ_REMOVE(&bts->actions, state, entry);
> - g_free(state);
> + put_blk_action_state(state);
> }
>
> + /* The implicit 'job' of the transaction itself is complete: */
> transaction_job_complete(bts);
> }
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 06/11] block: add refcount to Job object
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 06/11] block: add refcount to Job object John Snow
@ 2015-04-17 15:43 ` Max Reitz
0 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2015-04-17 15:43 UTC (permalink / raw)
To: John Snow, qemu-block; +Cc: kwolf, famz, qemu-devel, vsementsov, stefanha
On 27.03.2015 20:20, John Snow wrote:
> If we want to get at the job after the life of the job,
> we'll need a refcount for this object.
>
> This may occur for example if we wish to inspect the actions
> taken by a particular job after a transactional group of jobs
> runs, and further actions are required.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> blockjob.c | 18 ++++++++++++++++--
> include/block/blockjob.h | 21 +++++++++++++++++++++
> 2 files changed, 37 insertions(+), 2 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 07/11] block: add delayed bitmap successor cleanup
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 07/11] block: add delayed bitmap successor cleanup John Snow
@ 2015-04-17 15:49 ` Max Reitz
0 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2015-04-17 15:49 UTC (permalink / raw)
To: John Snow, qemu-block; +Cc: kwolf, famz, qemu-devel, vsementsov, stefanha
On 27.03.2015 20:20, John Snow wrote:
> Allow bitmap successors to carry reference counts.
>
> We can in a later patch use this ability to clean up the dirty bitmap
> according to both the individual job's success and the success of all
> jobs in the transaction group.
>
> The code for cleaning up a bitmap is also moved from backup_run to
> backup_complete.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> block.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++-----
> block/backup.c | 20 ++++++----------
> include/block/block.h | 10 ++++----
> 3 files changed, 70 insertions(+), 25 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 08/11] block: move transactions beneath qmp interfaces
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 08/11] block: move transactions beneath qmp interfaces John Snow
@ 2015-04-17 16:01 ` Max Reitz
2015-04-17 16:40 ` John Snow
2015-04-17 16:43 ` Eric Blake
0 siblings, 2 replies; 31+ messages in thread
From: Max Reitz @ 2015-04-17 16:01 UTC (permalink / raw)
To: John Snow, qemu-block; +Cc: kwolf, famz, qemu-devel, vsementsov, stefanha
On 27.03.2015 20:20, John Snow wrote:
> In general, since transactions may reference QMP function helpers,
> it would be nice for them to sit beneath them.
>
> This will avoid the need for forward declaring any QMP interfaces,
> which would be aggravating to update in so many places.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> blockdev.c | 2581 ++++++++++++++++++++++++++++++------------------------------
> 1 file changed, 1292 insertions(+), 1289 deletions(-)
I'm not so sure about this patch. I mean... 2581 changes? :-/
If you (or someone else) can convince me that forward declarations are
really worse than this (is it more aggravating than having a patch with
this diffcount?), well...
But even then, I'd strongly vote for removing dropping all functional
changes beside the code movement from this patch. Because I'm seeing this:
2096c2096,2097
< error_set(errp, QERR_INVALID_PARAMETER_VALUE, "granularity",
"power of 2");
---
> error_set(errp, QERR_INVALID_PARAMETER_VALUE,
> "granularity", "power of 2");
2517c2518,2519
< /* New and old BlockDriverState structs for atomic group operations */
---
>
/******************************************************************************/
> /* Transactions */
3439a3442,3443
>
>
/******************************************************************************/
Probably sensible changes, but if you really, really, really want to go
for this code movement, please apply them in an own patch before this
one so that this one really is nothing but code movement.
Max
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 09/11] qmp: Add an implementation wrapper for qmp_drive_backup
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 09/11] qmp: Add an implementation wrapper for qmp_drive_backup John Snow
@ 2015-04-17 16:12 ` Max Reitz
0 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2015-04-17 16:12 UTC (permalink / raw)
To: John Snow, qemu-block; +Cc: kwolf, famz, qemu-devel, vsementsov, stefanha
On 27.03.2015 20:20, John Snow wrote:
> We'd like to be able to specify the callback given to backup_start
> manually in the case of transactions, so split apart qmp_drive_backup
> into an implementation and a wrapper.
>
> Switch drive_backup_prepare to use the new wrapper, but don't overload
> the callback and closure yet.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> blockdev.c | 65 ++++++++++++++++++++++++++++++++++++++++++++------------------
> 1 file changed, 46 insertions(+), 19 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
The problem being that it relies on patch 8. I don't think being able to
avoid a single forward declaration warrants moving half (or more...) of
blockdev.c around, but I'm open to be convinced otherwise.
Max
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 08/11] block: move transactions beneath qmp interfaces
2015-04-17 16:01 ` Max Reitz
@ 2015-04-17 16:40 ` John Snow
2015-04-17 16:43 ` Eric Blake
1 sibling, 0 replies; 31+ messages in thread
From: John Snow @ 2015-04-17 16:40 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: kwolf, vsementsov, famz, qemu-devel, stefanha
On 04/17/2015 12:01 PM, Max Reitz wrote:
> On 27.03.2015 20:20, John Snow wrote:
>> In general, since transactions may reference QMP function helpers,
>> it would be nice for them to sit beneath them.
>>
>> This will avoid the need for forward declaring any QMP interfaces,
>> which would be aggravating to update in so many places.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>> blockdev.c | 2581
>> ++++++++++++++++++++++++++++++------------------------------
>> 1 file changed, 1292 insertions(+), 1289 deletions(-)
>
> I'm not so sure about this patch. I mean... 2581 changes? :-/
>
> If you (or someone else) can convince me that forward declarations are
> really worse than this (is it more aggravating than having a patch with
> this diffcount?), well...
>
> But even then, I'd strongly vote for removing dropping all functional
> changes beside the code movement from this patch. Because I'm seeing this:
>
> 2096c2096,2097
> < error_set(errp, QERR_INVALID_PARAMETER_VALUE, "granularity",
> "power of 2");
> ---
> > error_set(errp, QERR_INVALID_PARAMETER_VALUE,
> > "granularity", "power of 2");
> 2517c2518,2519
> < /* New and old BlockDriverState structs for atomic group operations */
> ---
> >
> /******************************************************************************/
>
> > /* Transactions */
> 3439a3442,3443
> >
> >
> /******************************************************************************/
>
>
> Probably sensible changes, but if you really, really, really want to go
> for this code movement, please apply them in an own patch before this
> one so that this one really is nothing but code movement.
>
> Max
>
OK. I fixed the line length because checkpatch yelled at me.
I can add a micropatch that adds my organizational flair and fixes line
lengths in a teensy patch that precedes this one.
Now: is the code movement necessary? well... Maybe, maybe not. I'd
actually like to just break out transactions into their own file
entirely, but that has its own challenges (like our heavy usage of
internal block goodies) ...
My only real justification is in the cover letter: forward declarations
of QMP functions is a burden.
Then again, so is basically erasing transaction development history...
:(
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 08/11] block: move transactions beneath qmp interfaces
2015-04-17 16:01 ` Max Reitz
2015-04-17 16:40 ` John Snow
@ 2015-04-17 16:43 ` Eric Blake
1 sibling, 0 replies; 31+ messages in thread
From: Eric Blake @ 2015-04-17 16:43 UTC (permalink / raw)
To: Max Reitz, John Snow, qemu-block
Cc: kwolf, vsementsov, famz, qemu-devel, stefanha
[-- Attachment #1: Type: text/plain, Size: 2042 bytes --]
On 04/17/2015 10:01 AM, Max Reitz wrote:
> On 27.03.2015 20:20, John Snow wrote:
>> In general, since transactions may reference QMP function helpers,
>> it would be nice for them to sit beneath them.
>>
>> This will avoid the need for forward declaring any QMP interfaces,
>> which would be aggravating to update in so many places.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>> blockdev.c | 2581
>> ++++++++++++++++++++++++++++++------------------------------
>> 1 file changed, 1292 insertions(+), 1289 deletions(-)
>
> I'm not so sure about this patch. I mean... 2581 changes? :-/
>
> If you (or someone else) can convince me that forward declarations are
> really worse than this (is it more aggravating than having a patch with
> this diffcount?), well...
I like avoiding forward declarations of static functions, where it makes
sense, but I'm not going to insist on reordering code if it makes things
ugly.
>
> But even then, I'd strongly vote for removing dropping all functional
> changes beside the code movement from this patch. Because I'm seeing this:
>
> 2096c2096,2097
> < error_set(errp, QERR_INVALID_PARAMETER_VALUE, "granularity",
> "power of 2");
> ---
>> error_set(errp, QERR_INVALID_PARAMETER_VALUE,
>> "granularity", "power of 2");
Indeed - you WANT code motion patches to be as easy as possible to
review. From http://wiki.qemu.org/Contribute/SubmitAPatch
"Ideally, a code motion patch can be reviewed by doing git format-patch
--stdout -1 > patch; diff -u <(sed -n 's/^-//p' patch) <(sed -n
's/^\+//p' patch), to focus on the few changes that weren't wholesale
code motion."
> Probably sensible changes, but if you really, really, really want to go
> for this code movement, please apply them in an own patch before this
> one so that this one really is nothing but code movement.
>
> Max
>
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 10/11] block: drive_backup transaction callback support
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 10/11] block: drive_backup transaction callback support John Snow
@ 2015-04-17 16:55 ` Max Reitz
0 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2015-04-17 16:55 UTC (permalink / raw)
To: John Snow, qemu-block; +Cc: kwolf, famz, qemu-devel, vsementsov, stefanha
On 27.03.2015 20:20, John Snow wrote:
> This patch actually implements the transactional callback system
> for the drive_backup transaction.
This line is probably not intended to be indented (I always wanted to
make that pun).
> (1) We manually pick up a reference to the bitmap if present to allow
> its cleanup to be delayed until after all drive_backup jobs launched
> by the transaction have fully completed.
>
> (2) We create a functional closure that envelops the original drive_backup
> callback, to be able to intercept the completion status and return code
> for the job.
>
> (3) We add the drive_backup_cb method for the drive_backup action, which
> unpacks the completion information and invokes the final cleanup.
>
> (4) backup_transaction_complete will perform the final cleanup on the
> backup job.
>
> (5) In the case of transaction cancellation, drive_backup_cb is still
> responsible for cleaning up the mess we may have already made.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> block/backup.c | 9 ++++++++
> blockdev.c | 52 ++++++++++++++++++++++++++++++++++++++++++++---
> include/block/block_int.h | 8 ++++++++
> 3 files changed, 66 insertions(+), 3 deletions(-)
With that fixed:
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 11/11] iotests: 124 - transactional failure test
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 11/11] iotests: 124 - transactional failure test John Snow
@ 2015-04-17 17:04 ` Max Reitz
0 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2015-04-17 17:04 UTC (permalink / raw)
To: John Snow, qemu-block; +Cc: kwolf, famz, qemu-devel, vsementsov, stefanha
On 27.03.2015 20:20, John Snow wrote:
> Use a transaction to request an incremental backup across two drives.
> Coerce one of the jobs to fail, and then re-run the transaction.
>
> Verify that no bitmap data was lost due to the partial transaction
> failure.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> tests/qemu-iotests/124 | 119 +++++++++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/124.out | 4 +-
> 2 files changed, 121 insertions(+), 2 deletions(-)
Just as patch 2, this will need fixup for v5 of the transaction-less series.
(the changes from v1 look good, though)
Max
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 05/11] block: add transactional callbacks feature
2015-04-17 15:41 ` Max Reitz
@ 2015-04-17 21:55 ` John Snow
0 siblings, 0 replies; 31+ messages in thread
From: John Snow @ 2015-04-17 21:55 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: kwolf, vsementsov, famz, qemu-devel, stefanha
On 04/17/2015 11:41 AM, Max Reitz wrote:
> On 27.03.2015 20:19, John Snow wrote:
>> The goal here is to add a new method to transactions that allows
>> developers to specify a callback that will get invoked only once
>> all jobs spawned by a transaction are completed, allowing developers
>> the chance to perform actions conditionally pending complete success,
>> partial failure, or complete failure.
>>
>> In order to register the new callback to be invoked, a user must request
>> a callback pointer and closure by calling new_action_cb_wrapper, which
>> creates a wrapper around an opaque pointer and callback that would have
>> originally been passed to e.g. backup_start().
>>
>> The function will return a function pointer and a new opaque pointer to
>> be passed instead. The transaction system will effectively intercept the
>> original callbacks and perform book-keeping on the transaction after it
>> has delivered the original enveloped callback.
>>
>> This means that Transaction Action callback methods will be called after
>> all callbacks triggered by all Actions in the Transactional group have
>> been received.
>>
>> This feature has no knowledge of any jobs spawned by Actions that do not
>> inform the system via new_action_cb_wrapper().
>>
>> For an example of how to use the feature, please skip ahead to:
>> 'block: drive_backup transaction callback support' which serves as an
>> example
>> for how to hook up a post-transaction callback to the Drive Backup
>> action.
>>
>>
>> Note 1: Defining a callback method alone is not sufficient to have the
>> new
>> method invoked. You must call new_action_cb_wrapper() AND
>> ensure the
>> callback it returns is the one used as the callback for the job
>> launched by the action.
>>
>> Note 2: You can use this feature for any system that registers
>> completions of
>> an asynchronous task via a callback of the form
>> (void *opaque, int ret), not just block job callbacks.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>> blockdev.c | 191
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 187 insertions(+), 4 deletions(-)
>
> I like it much better than v1! :-)
>
I do, too.
> Some minor comments below. (But just as in v1, I need to look into the
> following patches to know exactly how some of the functions introduced
> here are used)
>
>> diff --git a/blockdev.c b/blockdev.c
>> index f806d40..d404251 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -1239,6 +1239,8 @@ typedef struct BlkActionState BlkActionState;
>> * @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.
>> + * @cb: Executed after all jobs launched by actions in the
>> transaction finish,
>> + * but only if requested by new_action_cb_wrapper() prior to
>> clean().
>> *
>> * 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.
>> @@ -1249,6 +1251,7 @@ typedef struct BlkActionOps {
>> void (*commit)(BlkActionState *common);
>> void (*abort)(BlkActionState *common);
>> void (*clean)(BlkActionState *common);
>> + void (*cb)(BlkActionState *common);
>> } BlkActionOps;
>> /**
>> @@ -1257,19 +1260,46 @@ typedef struct BlkActionOps {
>> * by a transaction group.
>> *
>> * @jobs: A reference count that tracks how many jobs still need to
>> complete.
>> + * @status: A cumulative return code for all actions that have reported
>> + * a return code via callback in the transaction.
>> * @actions: A list of all Actions in the Transaction.
>> + * However, once the transaction has completed, it will be
>> only a list
>> + * of transactions that have registered a post-transaction
>> callback.
>> */
>> typedef struct BlkTransactionState {
>> int jobs;
>> + int status;
>> QTAILQ_HEAD(actions, BlkActionState) actions;
>> } BlkTransactionState;
>> +typedef void (CallbackFn)(void *opaque, int ret);
>> +
>> +/**
>> + * BlkActionCallbackData:
>> + * Necessary state for intercepting and
>> + * re-delivering a callback triggered by an Action.
>> + *
>> + * @opaque: The data to be given to the encapsulated callback when
>> + * a job launched by an Action completes.
>> + * @ret: The status code that was delivered to the encapsulated
>> callback.
>> + * @callback: The encapsulated callback to invoke upon completion of
>> + * the Job launched by the Action.
>> + */
>> +typedef struct BlkActionCallbackData {
>> + void *opaque;
>> + int ret;
>> + CallbackFn *callback;
>> +} BlkActionCallbackData;
>> +
>> /**
>> * 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.
>> + * @transaction: A pointer back to the Transaction this Action
>> belongs to.
>> + * @cb_data: Information on this Action's encapsulated callback, if any.
>> + * @refcount: reference count, allowing access to this state beyond
>> clean().
>> * @entry: List membership for all Actions in this Transaction.
>> *
>> * This structure must be arranged as first member in a subclassed
>> type,
>> @@ -1279,6 +1309,9 @@ typedef struct BlkTransactionState {
>> struct BlkActionState {
>> TransactionAction *action;
>> const BlkActionOps *ops;
>> + BlkTransactionState *transaction;
>> + BlkActionCallbackData *cb_data;
>> + int refcount;
>> QTAILQ_ENTRY(BlkActionState) entry;
>> };
>> @@ -1294,6 +1327,26 @@ static BlkTransactionState
>> *new_blk_transaction_state(void)
>> return bts;
>> }
>> +static BlkActionState *put_blk_action_state(BlkActionState *state)
>> +{
>> + BlkActionState *bas = state;
>
> I don't think this variable needs to be initialized.
>
Or just not needed at all, as you point out.
>> +
>> + state->refcount--;
>> + if (state->refcount == 0) {
>> +
>
> Superfluous empty line.
>
As an American, I like wide open spaces.
>> + QTAILQ_FOREACH(bas, &state->transaction->actions, entry) {
>> + if (bas == state) {
>> + QTAILQ_REMOVE(&state->transaction->actions, bas, entry);
>> + break;
>> + }
>> + }
>
> Wouldn't QTAILQ_REMOVE(&state->transaction->actions, state, entry); do
> exactly the same thing as this loop? There'd be a difference only if
> "state" is not element of the list of actions, but think that should be
> impossible.
>
> Max
>
It's a holdover from the earlier revision where there were two lists (a
list of completed items and a list of all items), because QTAILQ_REMOVE
does not properly accommodate the case where the key to remove is not
present within the list.
Since I unified the lists, though, there's no confusion about which
actions are in which lists. It's now just a superfluous safety dance.
>> + g_free(state->cb_data);
>> + g_free(state);
>> + return NULL;
>> + }
>> + return state;
>> +}
>> +
>> static void destroy_blk_transaction_state(BlkTransactionState *bts)
>> {
>> BlkActionState *bas, *bas_next;
>> @@ -1301,23 +1354,151 @@ static void
>> destroy_blk_transaction_state(BlkTransactionState *bts)
>> /* The list should in normal cases be empty,
>> * but in case someone really just wants to kibosh the whole
>> deal: */
>> QTAILQ_FOREACH_SAFE(bas, &bts->actions, entry, bas_next) {
>> - QTAILQ_REMOVE(&bts->actions, bas, entry);
>> - g_free(bas);
>> + put_blk_action_state(bas);
>> }
>> g_free(bts);
>> }
>> +static void transaction_run_cb_action_ops(BlkTransactionState *bts)
>> +{
>> + BlkActionState *bas, *bas_next;
>> +
>> + QTAILQ_FOREACH_SAFE(bas, &bts->actions, entry, bas_next) {
>> + if (bas->ops->cb) {
>> + bas->ops->cb(bas);
>> + }
>> +
>> + g_free(bas->cb_data);
>> + bas->cb_data = NULL;
>> + put_blk_action_state(bas);
>> + }
>> +}
>> +
>> static BlkTransactionState
>> *transaction_job_complete(BlkTransactionState *bts)
>> {
>> bts->jobs--;
>> if (bts->jobs == 0) {
>> + transaction_run_cb_action_ops(bts);
>> destroy_blk_transaction_state(bts);
>> return NULL;
>> }
>> return bts;
>> }
>> +static void transaction_job_cancel(BlkActionState *bas)
>> +{
>> + assert(bas->transaction->jobs > 1);
>> + transaction_job_complete(bas->transaction);
>> +}
>> +
>> +/**
>> + * Intercept a callback that was issued due to a transactional action.
>> + */
>> +static void transaction_action_callback(void *opaque, int ret)
>> +{
>> + BlkActionState *common = opaque;
>> + BlkActionCallbackData *cb_data = common->cb_data;
>> + CallbackFn *callback = cb_data->callback;
>> +
>> + /* Prepare data for ops->cb() */
>> + cb_data->callback = NULL;
>> + cb_data->ret = ret;
>> +
>> + /* Cumulative return code will track the first error discovered */
>> + if (!common->transaction->status) {
>> + common->transaction->status = ret;
>> + }
>> +
>> + /* Deliver the intercepted callback prior to marking it as
>> completed */
>> + callback(cb_data->opaque, cb_data->ret);
>> + transaction_job_complete(common->transaction);
>> +}
>> +
>> +/**
>> + * Create a new transactional callback wrapper.
>> + *
>> + * Given a callback and a closure, generate a new
>> + * callback and closure that will invoke the
>> + * given callback with the given closure.
>> + *
>> + * After all wrappers in the transactional group have
>> + * been processed, each action's .cb() method will be
>> + * invoked.
>> + *
>> + * @common The transaction action state to set a callback for.
>> + * @opaque A closure intended for the encapsulated callback.
>> + * @callback The callback we are encapsulating.
>> + * @new_opaque[out]: The closure to be used instead of @opaque.
>> + *
>> + * @return The callback to be used instead of @callback.
>> + */
>> +__attribute__((__unused__))
>> +static CallbackFn *new_action_cb_wrapper(BlkActionState *common,
>> + void *opaque,
>> + CallbackFn *callback,
>> + void **new_opaque)
>> +{
>> + BlkActionCallbackData *cb_data = g_new0(BlkActionCallbackData, 1);
>> + assert(new_opaque);
>> +
>> + /* Stash the original callback information */
>> + cb_data->opaque = opaque;
>> + cb_data->callback = callback;
>> + common->cb_data = cb_data;
>> +
>> + /* The BAS itself will serve as our new closure */
>> + *new_opaque = common;
>> + common->refcount++;
>> +
>> + /* Inform the transaction to expect one more return */
>> + common->transaction->jobs++;
>> +
>> + /* Lastly, the actual callback function to handle the
>> interception. */
>> + return transaction_action_callback;
>> +}
>> +
>> +/**
>> + * Undo any actions performed by the above call.
>> + */
>> +__attribute__((__unused__))
>> +static void cancel_action_cb_wrapper(BlkActionState *common)
>> +{
>> + /* Stage 0: Wrapper was never created: */
>> + if (common->cb_data == NULL) {
>> + assert(common->refcount == 1);
>> + return;
>> + }
>> +
>> + /* Stage 1: Callback was created, but the job never launched.
>> + * (We assume that the job cannot possibly be running, because
>> + * we assume it has been synchronously canceled if it was.)
>> + *
>> + * We also assume that any cleanup normally handled by cb()
>> + * is not needed, because it should have been prepared after
>> + * the job launched.
>> + */
>> + if (common->cb_data && common->cb_data->callback) {
>> + transaction_job_cancel(common);
>> + goto cleanup;
>> + }
>> +
>> + /* Stage 2: Job already completed, or was canceled.
>> + *
>> + * Force an error in the callback data and just invoke the
>> completion
>> + * handler to perform appropriate cleanup for us.
>> + */
>> + if (common->cb_data && !common->cb_data->callback) {
>> + common->cb_data->ret = -ECANCELED;
>> + common->ops->cb(common);
>> + }
>> +
>> + cleanup:
>> + g_free(common->cb_data);
>> + common->cb_data = NULL;
>> + put_blk_action_state(common);
>> +}
>> +
>> /* internal snapshot private data */
>> typedef struct InternalSnapshotState {
>> BlkActionState common;
>> @@ -1927,8 +2108,10 @@ void qmp_transaction(TransactionActionList
>> *dev_list, Error **errp)
>> assert(ops->instance_size > 0);
>> state = g_malloc0(ops->instance_size);
>> + state->refcount = 1;
>> state->ops = ops;
>> state->action = dev_info;
>> + state->transaction = bts;
>> QTAILQ_INSERT_TAIL(&bts->actions, state, entry);
>> state->ops->prepare(state, &local_err);
>> @@ -1959,10 +2142,10 @@ exit:
>> if (state->ops->clean) {
>> state->ops->clean(state);
>> }
>> - QTAILQ_REMOVE(&bts->actions, state, entry);
>> - g_free(state);
>> + put_blk_action_state(state);
>> }
>> + /* The implicit 'job' of the transaction itself is complete: */
>> transaction_job_complete(bts);
>> }
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH v2.5 00/10] block: incremental backup transactions
2015-03-27 19:19 [Qemu-devel] [PATCH v2 00/11] block: incremental backup transactions John Snow
` (10 preceding siblings ...)
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 11/11] iotests: 124 - transactional failure test John Snow
@ 2015-04-18 1:01 ` John Snow
2015-04-21 13:53 ` Kashyap Chamarthy
11 siblings, 1 reply; 31+ messages in thread
From: John Snow @ 2015-04-18 1:01 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, famz, qemu-devel, mreitz, vsementsov, stefanha
I am not prepared to send a v3 on this, primarily because I am still
waffling on whether or not to do the code motion patch that is present
in patch #8 of v2 of this series.
However, for the purposes of testing, reviewers may find it convenient
to have a new version of this series that applies cleanly in concert
with v6 of the transactionless series, so I am presenting an informal
"version 2.5" of this series.
Patches are available on github:
https://github.com/jnsnow/qemu/commits/incremental-transactions
And here is the v2.5 patchset diff:
Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences,
respectively
001/10:[0002] [FC] 'qapi: Add transaction support to block-dirty-bitmap
operations'
002/10:[0021] [FC] 'iotests: add transactional incremental backup test'
003/10:[----] [--] 'block: rename BlkTransactionState and BdrvActionOps'
004/10:[----] [--] 'block: re-add BlkTransactionState'
005/10:[0010] [FC] 'block: add transactional callbacks feature'
006/10:[----] [--] 'block: add refcount to Job object'
007/10:[----] [-C] 'block: add delayed bitmap successor cleanup'
008/10:[0051] [FC] 'qmp: Add an implementation wrapper for qmp_drive_backup'
009/10:[0001] [FC] 'block: drive_backup transaction callback support'
010/10:[0033] [FC] 'iotests: 124 - transactional failure test'
01: Removed "Not yet implemented" line from bitmaps.md
02: Rebase fallout
05: Removed superfluous deletion loop pointed out by Max
08: Fallout from removing the code motion patch,
new forward declaration.
09: Fallout from removing the code motion patch,
new forward declaration.
10: Fallout from the rebase.
There are some more changes that need to happen to #10, primarily a bit
more refactoring after the changes made to how images are checked.
On 03/27/2015 03:19 PM, John Snow wrote:
> requires: 1426879023-18151-1-git-send-email-jsnow@redhat.com
> "[PATCH v4 00/20] block: transactionless incremental backup series"
>
> This series adds support for incremental backup primitives in QMP
> transactions. It requires my transactionless incremental backup series,
> currently at v4.
>
> Patch 1 adds basic support for add and clear transactions.
> Patch 2 tests this basic support.
> Patches 3-4 refactor transactions a little bit, to add clarity.
> Patch 5 adds the framework for error scenarios where only
> some jobs that were launched by a transaction complete successfully,
> and we need to perform context-sensitive cleanup after the transaction
> itself has already completed.
> Patches 6-7 add necessary bookkeeping information to backup job
> data structures to take advantage of this new feature.
> Patch 8 just moves code.
> Patch 9 modifies qmp_drive_backup to support the new feature.
> Patch 10 implements the new feature for drive_backup transaction actions.
> Patch 11 tests the new feature.
>
> Lingering questions:
> - Is it worth it to add a post-transaction completion event to QMP that
> signifies all jobs launched by the transaction have completed? This
> would be of primary interest to libvirt, in particular, but only as
> a convenience feature.
>
> Thank you,
> --John Snow
>
> v2:
>
> Key:
> [----] : patches are identical
> [####] : number of functional differences between upstream/downstream patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
>
> 001/11:[0009] [FC] 'qapi: Add transaction support to block-dirty-bitmap operations'
> 002/11:[0036] [FC] 'iotests: add transactional incremental backup test'
> 003/11:[down] 'blockdev: rename BlkTransactionState and BdrvActionOps'
> 004/11:[down] 'block: re-add BlkTransactionState'
> 005/11:[0274] [FC] 'block: add transactional callbacks feature'
> 006/11:[0004] [FC] 'block: add refcount to Job object'
> 007/11:[0048] [FC] 'block: add delayed bitmap successor cleanup'
> 008/11:[down] 'block: move transactions beneath qmp interfaces'
> 009/11:[0084] [FC] 'qmp: Add an implementation wrapper for qmp_drive_backup'
> 010/11:[0050] [FC] 'block: drive_backup transaction callback support'
> 011/11:[0004] [FC] 'iotests: 124 - transactional failure test'
>
> 01: Fixed indentation.
> Fixed QMP commands to behave with new bitmap_lookup from
> transactionless-v4.
> 2.3 --> 2.4.
> 02: Folded in improvements to qemu-iotest 124 from transactional-v1.
> 03: NEW
> 04: NEW
> 05: A lot:
> Don't delete the comma in the transaction actions config
> use g_new0 instead of g_malloc0
> Phrasing: "retcode" --> "Return code"
> Use GCC attributes to mark functions as unused until future patches.
> Added some data structure documentation.
> Many structure and function renames, hopefully to improve readability.
> Use just one list for all Actions instead of two separate lists.
> Remove ActionState from the list upon deletion/decref
> And many other small tweaks.
> 06: Comment phrasing.
> 07: Removed errp parameter from all functions introduced by this commit.
> bdrv_dirty_bitmap_decref --> bdrv_frozen_bitmap_decref
> 08: NEW
> 09: _drive_backup --> do_drive_backup()
> Forward declarations removed.
> 10: Rebased on top of drastically modified #05.
> Phrasing: "BackupBlockJob" instead of "BackupJob" in comments.
> 11: Removed extra parameters to wait_incremental() in
> test_transaction_failure()
>
> John Snow (11):
> qapi: Add transaction support to block-dirty-bitmap operations
> iotests: add transactional incremental backup test
> block: rename BlkTransactionState and BdrvActionOps
> block: re-add BlkTransactionState
> block: add transactional callbacks feature
> block: add refcount to Job object
> block: add delayed bitmap successor cleanup
> block: move transactions beneath qmp interfaces
> qmp: Add an implementation wrapper for qmp_drive_backup
> block: drive_backup transaction callback support
> iotests: 124 - transactional failure test
>
> block.c | 65 +-
> block/backup.c | 29 +-
> blockdev.c | 1599 ++++++++++++++++++++++++++++----------------
> blockjob.c | 18 +-
> include/block/block.h | 10 +-
> include/block/block_int.h | 8 +
> include/block/blockjob.h | 21 +
> qapi-schema.json | 6 +-
> tests/qemu-iotests/124 | 170 +++++
> tests/qemu-iotests/124.out | 4 +-
> 10 files changed, 1313 insertions(+), 617 deletions(-)
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2.5 00/10] block: incremental backup transactions
2015-04-18 1:01 ` [Qemu-devel] [PATCH v2.5 00/10] block: incremental backup transactions John Snow
@ 2015-04-21 13:53 ` Kashyap Chamarthy
2015-04-21 14:48 ` Kashyap Chamarthy
2015-04-21 20:33 ` Kashyap Chamarthy
0 siblings, 2 replies; 31+ messages in thread
From: Kashyap Chamarthy @ 2015-04-21 13:53 UTC (permalink / raw)
To: John Snow
Cc: kwolf, famz, qemu-block, qemu-devel, mreitz, vsementsov, stefanha
On Fri, Apr 17, 2015 at 09:01:10PM -0400, John Snow wrote:
> I am not prepared to send a v3 on this, primarily because I am still
> waffling on whether or not to do the code motion patch that is present in
> patch #8 of v2 of this series.
>
> However, for the purposes of testing, reviewers may find it convenient to
> have a new version of this series that applies cleanly in concert with v6 of
> the transactionless series, so I am presenting an informal "version 2.5" of
> this series.
>
> Patches are available on github:
> https://github.com/jnsnow/qemu/commits/incremental-transactions
I just tested this w/o creating the target image, and I don't see any
error thrown.
Details:
I tested with this branch:
https://github.com/jnsnow/qemu/commits/incremental-transactions
$ git checkout -b v2.5-and-v6-inc-backup jsnow/incremental-transactions
$ git describe
v2.3.0-rc3-37-g6932a32
With a QEMU invocation (of a CirrOS disk image) with QMP server:
. . .
char device redirected to /dev/pts/31 (label charserial0)
QEMU waiting for connection on: disconnected:tcp:localhost:4444,server
And, invoking `drive-backup` *without* pre-creating the target image
(i.e. 'incremental.o.img'):
{ 'execute': 'drive-backup',
'arguments': {
'device': 'ide0-0-0',
'bitmap': 'bitmap0',
'sync': 'dirty-bitmap',
'target': 'incremental.0.img',
'mode': 'existing',
'format': 'qcow2'
}
}
Results in:
{"QMP": {"version": {"qemu": {"micro": 93, "minor": 2, "major": 2},
"package": ""}, "capabilities": []}} {"return": {}}
Instead of an "error". Is this a bug?
On a related note, Kevin Wolf said on IRC that he gets an error (without
'bitmap') when tested from QEMU master branch.
--
/kashyap
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2.5 00/10] block: incremental backup transactions
2015-04-21 13:53 ` Kashyap Chamarthy
@ 2015-04-21 14:48 ` Kashyap Chamarthy
2015-04-21 20:33 ` Kashyap Chamarthy
1 sibling, 0 replies; 31+ messages in thread
From: Kashyap Chamarthy @ 2015-04-21 14:48 UTC (permalink / raw)
To: John Snow
Cc: kwolf, famz, qemu-block, qemu-devel, mreitz, vsementsov, stefanha
On Tue, Apr 21, 2015 at 03:53:11PM +0200, Kashyap Chamarthy wrote:
[. . .]
> And, invoking `drive-backup` *without* pre-creating the target image
> (i.e. 'incremental.o.img'):
>
> { 'execute': 'drive-backup',
> 'arguments': {
> 'device': 'ide0-0-0',
Small typo (in this email, not in my test): s/ide0-0-0/drive-ide0-0-0
> 'bitmap': 'bitmap0',
> 'sync': 'dirty-bitmap',
> 'target': 'incremental.0.img',
> 'mode': 'existing',
> 'format': 'qcow2'
> }
> }
>
> Results in:
>
> {"QMP": {"version": {"qemu": {"micro": 93, "minor": 2, "major": 2},
> "package": ""}, "capabilities": []}} {"return": {}}
>
> Instead of an "error". Is this a bug?
>
> On a related note, Kevin Wolf said on IRC that he gets an error
> (without 'bitmap') when tested from QEMU master branch.
On a separate machine, I tested the above behavior with master
(f2a5810), but I don't see any error, without 'bitmap' and with 'mode'
as 'existing', i.e.:
{ 'execute': 'drive-backup', 'arguments':
{ 'device': 'drive-virtio-disk0', 'sync': 'full', 'target':
'/var/lib/libvirt/images/incremental.0.img', 'mode': 'existing', 'format': 'qcow2' } }
still results in:
{"return": {}}
Maybe I doing something wrong?
--
/kashyap
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2.5 00/10] block: incremental backup transactions
2015-04-21 13:53 ` Kashyap Chamarthy
2015-04-21 14:48 ` Kashyap Chamarthy
@ 2015-04-21 20:33 ` Kashyap Chamarthy
1 sibling, 0 replies; 31+ messages in thread
From: Kashyap Chamarthy @ 2015-04-21 20:33 UTC (permalink / raw)
To: John Snow
Cc: kwolf, famz, qemu-block, qemu-devel, mreitz, vsementsov, stefanha
On Tue, Apr 21, 2015 at 03:53:11PM +0200, Kashyap Chamarthy wrote:
[. . .]
> And, invoking `drive-backup` *without* pre-creating the target image
> (i.e. 'incremental.o.img'):
>
> { 'execute': 'drive-backup',
> 'arguments': {
> 'device': 'ide0-0-0',
> 'bitmap': 'bitmap0',
> 'sync': 'dirty-bitmap',
> 'target': 'incremental.0.img',
> 'mode': 'existing',
> 'format': 'qcow2'
> }
> }
>
> Results in:
>
> {"QMP": {"version": {"qemu": {"micro": 93, "minor": 2, "major": 2},
> "package": ""}, "capabilities": []}} {"return": {}}
>
> Instead of an "error". Is this a bug?
Answering myself, no, it's not.
I learnt from John Snow and Eric Blake that I needed additional 'reads'
to see event lines in my trivial QMP shell script[1] -- "because QEMU
sends events as the job makes progress, when you first connect, the qemu
sends a line BEFORE you send your 'qmp_capabilities' response so you're
off by one if you don't read the initial server greeting" (detail
by Eric).
A minimal test below:
(1) This time, invoke QEMU w/ QMP server over Unix socket:
---------------
$ ./invoke-qemu-with-qmp.sh
. . .
-qmp unix:./qmp-sock,server
char device redirected to /dev/pts/49 (label charserial0)
QEMU waiting for connection on: disconnected:unix:./qmp-sock,server
---------------
(2) Pre-create the target destination:
$ qemu-img create -f qcow2 incremental.0.img -b full_backup.img -F qcow2
(3) And, use `rlwrap` in combination with `socat` (previously learnt
this trick from Markus Armbruster, it conveniently retains the command
history) to connect to the QMP server and issue 'block-dirty-bitmap-add'
and 'drive-mirror' commands with 'dirty-bitmap':
----------------
$ rlwrap -H ~/.qmp_history socat UNIX-CONNECT:./qmp-sock STDIO
{"QMP": {"version": {"qemu": {"micro": 93, "minor": 2, "major": 2}, "package": ""}, "capabilities": []}}
{"execute":"qmp_capabilities"}
{"return": {}}
{ 'execute': 'block-dirty-bitmap-add', 'arguments': { 'node': 'drive-ide0-0-0', 'name': 'bitmap0' } }
{"return": {}}
{ 'execute': 'drive-backup', 'arguments': { 'device': 'drive-ide0-0-0', 'bitmap': 'bitmap0', 'sync': 'dirty-bitmap', 'target': '/home/kashyapc/work/virt/qemu/incremental-backup-test-qemu/tests/incremental.0.img', 'mode': 'existing', 'format': 'qcow2' } }
{"return": {}}
{"timestamp": {"seconds": 1429647518, "microseconds": 663755}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "drive-ide0-0-0", "len": 41126400, "offset": 41156608, "speed": 0, "type": "backup"}}
----------------
Thanks, John/Eric.
[1] Discussion thread here which contains the trivial script used:
http://lists.nongnu.org/archive/html/qemu-devel/2015-04/msg02432.html
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2015-04-21 20:34 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-27 19:19 [Qemu-devel] [PATCH v2 00/11] block: incremental backup transactions John Snow
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 01/11] qapi: Add transaction support to block-dirty-bitmap operations John Snow
2015-04-17 14:41 ` Max Reitz
2015-04-17 14:50 ` Eric Blake
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 02/11] iotests: add transactional incremental backup test John Snow
2015-04-17 14:42 ` Max Reitz
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 03/11] block: rename BlkTransactionState and BdrvActionOps John Snow
2015-04-17 14:51 ` Max Reitz
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 04/11] block: re-add BlkTransactionState John Snow
2015-04-17 15:11 ` Max Reitz
2015-03-27 19:19 ` [Qemu-devel] [PATCH v2 05/11] block: add transactional callbacks feature John Snow
2015-04-17 15:41 ` Max Reitz
2015-04-17 21:55 ` John Snow
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 06/11] block: add refcount to Job object John Snow
2015-04-17 15:43 ` Max Reitz
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 07/11] block: add delayed bitmap successor cleanup John Snow
2015-04-17 15:49 ` Max Reitz
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 08/11] block: move transactions beneath qmp interfaces John Snow
2015-04-17 16:01 ` Max Reitz
2015-04-17 16:40 ` John Snow
2015-04-17 16:43 ` Eric Blake
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 09/11] qmp: Add an implementation wrapper for qmp_drive_backup John Snow
2015-04-17 16:12 ` Max Reitz
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 10/11] block: drive_backup transaction callback support John Snow
2015-04-17 16:55 ` Max Reitz
2015-03-27 19:20 ` [Qemu-devel] [PATCH v2 11/11] iotests: 124 - transactional failure test John Snow
2015-04-17 17:04 ` Max Reitz
2015-04-18 1:01 ` [Qemu-devel] [PATCH v2.5 00/10] block: incremental backup transactions John Snow
2015-04-21 13:53 ` Kashyap Chamarthy
2015-04-21 14:48 ` Kashyap Chamarthy
2015-04-21 20:33 ` Kashyap Chamarthy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).