From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49204) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZC7KD-0004d8-2M for qemu-devel@nongnu.org; Mon, 06 Jul 2015 10:24:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZC7KB-0001RK-0P for qemu-devel@nongnu.org; Mon, 06 Jul 2015 10:24:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37089) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZC7KA-0001QM-IE for qemu-devel@nongnu.org; Mon, 06 Jul 2015 10:24:46 -0400 From: Stefan Hajnoczi Date: Mon, 6 Jul 2015 15:24:26 +0100 Message-Id: <1436192669-10062-8-git-send-email-stefanha@redhat.com> In-Reply-To: <1436192669-10062-1-git-send-email-stefanha@redhat.com> References: <1436192669-10062-1-git-send-email-stefanha@redhat.com> Subject: [Qemu-devel] [PATCH v2 07/10] block/backup: support block job transactions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Kevin Wolf , famz@redhat.com, John Snow , Jeff Cody , mreitz@redhat.com, vsementsov@parallels.com, Stefan Hajnoczi Join the transaction when the 'transactional-cancel' QMP argument is true. This ensures that the sync bitmap is not thrown away if another block job in the transaction is cancelled or fails. This is critical so incremental backup with multiple disks can be retried in case of cancellation/failure. Signed-off-by: Stefan Hajnoczi --- v2: * Convert blockdev-backup in addition to drive-backup * Add 'transactional-cancel' argument so users can enable/disable transactional behavior. This preserves semantics for existing users and allows fine-grained control over when to use transaction semantics. [jsnow] --- block/backup.c | 5 +- blockdev.c | 139 ++++++++++++++++++++++++++++++++++++---------- hmp.c | 2 +- include/block/block_int.h | 3 +- qapi/block-core.json | 14 ++++- 5 files changed, 130 insertions(+), 33 deletions(-) diff --git a/block/backup.c b/block/backup.c index 965654d..c31cd3d 100644 --- a/block/backup.c +++ b/block/backup.c @@ -429,6 +429,8 @@ static void coroutine_fn backup_run(void *opaque) qemu_co_rwlock_wrlock(&job->flush_rwlock); qemu_co_rwlock_unlock(&job->flush_rwlock); + block_job_txn_job_done(job->common.txn, &job->common, ret); + if (job->sync_bitmap) { BdrvDirtyBitmap *bm; if (ret < 0 || block_job_is_cancelled(&job->common)) { @@ -457,7 +459,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target, BlockdevOnError on_source_error, BlockdevOnError on_target_error, BlockCompletionFunc *cb, void *opaque, - Error **errp) + BlockJobTxn *txn, Error **errp) { int64_t len; @@ -539,6 +541,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target, sync_bitmap : NULL; job->common.len = len; job->common.co = qemu_coroutine_create(backup_run); + block_job_txn_add_job(txn, &job->common); qemu_coroutine_enter(job->common.co, job); return; diff --git a/blockdev.c b/blockdev.c index 9a83f8b..c8a4e43 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1586,12 +1586,25 @@ typedef struct DriveBackupState { BlockJob *job; } DriveBackupState; +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, + BlockJobTxn *txn, Error **errp); + static void drive_backup_prepare(BlkActionState *common, Error **errp) { DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common); BlockDriverState *bs; BlockBackend *blk; DriveBackup *backup; + BlockJobTxn *txn = NULL; Error *local_err = NULL; assert(common->action->kind == TRANSACTION_ACTION_KIND_DRIVE_BACKUP); @@ -1609,15 +1622,20 @@ 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); + if (backup->has_transactional_cancel && + backup->transactional_cancel) { + txn = common->block_job_txn; + } + + 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, + txn, &local_err); if (local_err) { error_propagate(errp, local_err); return; @@ -1654,12 +1672,22 @@ typedef struct BlockdevBackupState { AioContext *aio_context; } BlockdevBackupState; +static void do_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, + BlockJobTxn *txn, Error **errp); + static void blockdev_backup_prepare(BlkActionState *common, Error **errp) { BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common); BlockdevBackup *backup; BlockDriverState *bs, *target; BlockBackend *blk; + BlockJobTxn *txn = NULL; Error *local_err = NULL; assert(common->action->kind == TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP); @@ -1688,12 +1716,17 @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp) } aio_context_acquire(state->aio_context); - qmp_blockdev_backup(backup->device, backup->target, - backup->sync, - backup->has_speed, backup->speed, - backup->has_on_source_error, backup->on_source_error, - backup->has_on_target_error, backup->on_target_error, - &local_err); + if (backup->has_transactional_cancel && + backup->transactional_cancel) { + txn = common->block_job_txn; + } + + do_blockdev_backup(backup->device, backup->target, + backup->sync, + backup->has_speed, backup->speed, + backup->has_on_source_error, backup->on_source_error, + backup->has_on_target_error, backup->on_target_error, + txn, &local_err); if (local_err) { error_propagate(errp, local_err); return; @@ -2582,15 +2615,17 @@ 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, + BlockJobTxn *txn, Error **errp) { BlockBackend *blk; BlockDriverState *bs; @@ -2707,7 +2742,7 @@ void qmp_drive_backup(const char *device, const char *target, backup_start(bs, target_bs, speed, sync, bmap, on_source_error, on_target_error, - block_job_cb, bs, &local_err); + block_job_cb, bs, txn, &local_err); if (local_err != NULL) { bdrv_unref(target_bs); error_propagate(errp, local_err); @@ -2718,19 +2753,44 @@ 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, + bool has_transactional_cancel, bool transactional_cancel, + Error **errp) +{ + if (has_transactional_cancel && transactional_cancel) { + error_setg(errp, "Transactional cancel can only be used in the " + "'transaction' command"); + return; + } + + return 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, errp); +} + BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp) { return bdrv_named_nodes_list(errp); } -void qmp_blockdev_backup(const char *device, const char *target, +void do_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) + BlockJobTxn *txn, Error **errp) { BlockBackend *blk; BlockDriverState *bs; @@ -2768,7 +2828,7 @@ void qmp_blockdev_backup(const char *device, const char *target, 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); + on_target_error, block_job_cb, bs, txn, &local_err); if (local_err != NULL) { bdrv_unref(target_bs); error_propagate(errp, local_err); @@ -2777,6 +2837,29 @@ out: aio_context_release(aio_context); } +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, + bool has_transactional_cancel, + bool transactional_cancel, + Error **errp) +{ + if (has_transactional_cancel && transactional_cancel) { + error_setg(errp, "Transactional cancel can only be used in the " + "'transaction' command"); + return; + } + + do_blockdev_backup(device, target, sync, has_speed, speed, + has_on_source_error, on_source_error, + has_on_target_error, on_target_error, + NULL, errp); +} + #define DEFAULT_MIRROR_BUF_SIZE (10 << 20) void qmp_drive_mirror(const char *device, const char *target, diff --git a/hmp.c b/hmp.c index dcc66f1..f492965 100644 --- a/hmp.c +++ b/hmp.c @@ -1090,7 +1090,7 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict) qmp_drive_backup(device, filename, !!format, format, full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP, true, mode, false, 0, false, NULL, - false, 0, false, 0, &err); + false, 0, false, 0, false, false, &err); hmp_handle_error(mon, &err); } diff --git a/include/block/block_int.h b/include/block/block_int.h index 6eb22c7..5382422 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -642,6 +642,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target, * @on_target_error: The action to take upon error writing to the target. * @cb: Completion function for the job. * @opaque: Opaque pointer value passed to @cb. + * @txn: Transaction that this job is part of (may be NULL). * * Start a backup operation on @bs. Clusters in @bs are written to @target * until the job is cancelled or manually completed. @@ -652,7 +653,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target, BlockdevOnError on_source_error, BlockdevOnError on_target_error, BlockCompletionFunc *cb, void *opaque, - Error **errp); + BlockJobTxn *txn, Error **errp); void blk_dev_change_media_cb(BlockBackend *blk, bool load); bool blk_dev_has_removable_media(BlockBackend *blk); diff --git a/qapi/block-core.json b/qapi/block-core.json index 7b2efb8..d5e33fd 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -736,6 +736,10 @@ # default 'report' (no limitations, since this applies to # a different block device than @device). # +# @transactional-cancel: #optional whether failure or cancellation of other +# block jobs with @transactional-cancel true causes the +# whole group to cancel. +# # Note that @on-source-error and @on-target-error only affect background I/O. # If an error occurs during a guest write request, the device's rerror/werror # actions will be used. @@ -747,7 +751,8 @@ 'sync': 'MirrorSyncMode', '*mode': 'NewImageMode', '*speed': 'int', '*bitmap': 'str', '*on-source-error': 'BlockdevOnError', - '*on-target-error': 'BlockdevOnError' } } + '*on-target-error': 'BlockdevOnError', + '*transactional-cancel': 'bool' } } ## # @BlockdevBackup @@ -771,6 +776,10 @@ # default 'report' (no limitations, since this applies to # a different block device than @device). # +# @transactional-cancel: #optional whether failure or cancellation of other +# block jobs with @transactional-cancel true causes the +# whole group to cancel. +# # Note that @on-source-error and @on-target-error only affect background I/O. # If an error occurs during a guest write request, the device's rerror/werror # actions will be used. @@ -782,7 +791,8 @@ 'sync': 'MirrorSyncMode', '*speed': 'int', '*on-source-error': 'BlockdevOnError', - '*on-target-error': 'BlockdevOnError' } } + '*on-target-error': 'BlockdevOnError', + '*transactional-cancel': 'bool' } } ## # @blockdev-snapshot-sync -- 2.4.3