@@ -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);
@@ -1426,7 +1426,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,
@@ -1454,7 +1453,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: */
@@ -1805,6 +1803,7 @@ static void do_drive_backup(const char *device, const char *target,
BlockdevOnError on_target_error,
BlockCompletionFunc *cb, void *opaque,
Error **errp);
+static void block_job_cb(void *opaque, int ret);
static void drive_backup_prepare(BlkActionState *common, Error **errp)
{
@@ -1813,6 +1812,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;
@@ -1824,6 +1826,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);
@@ -1836,7 +1851,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);
@@ -1845,6 +1860,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)
@@ -1856,6 +1877,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)
@@ -1867,6 +1892,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;
@@ -2054,6 +2100,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),
@@ -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);
This patch actually implements the transactional callback system for the drive_backup action. (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 | 53 ++++++++++++++++++++++++++++++++++++++++++++--- include/block/block_int.h | 8 +++++++ 3 files changed, 67 insertions(+), 3 deletions(-)