All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Fam Zheng <famz@redhat.com>
To: qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	famz@redhat.com, John Snow <jsnow@redhat.com>,
	Jeff Cody <jcody@redhat.com>, Max Reitz <mreitz@redhat.com>,
	vsementsov@parallels.com, stefanha@redhat.com
Subject: [Qemu-devel] [PATCH v3 12/15] block/backup: support block job transactions
Date: Fri, 10 Jul 2015 11:46:49 +0800	[thread overview]
Message-ID: <1436500012-32593-13-git-send-email-famz@redhat.com> (raw)
In-Reply-To: <1436500012-32593-1-git-send-email-famz@redhat.com>

From: Stefan Hajnoczi <stefanha@redhat.com>

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 <stefanha@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/backup.c            |  23 +++++++-
 blockdev.c                | 139 ++++++++++++++++++++++++++++++++++++----------
 hmp.c                     |   2 +-
 include/block/block_int.h |   3 +-
 qapi/block-core.json      |  14 ++++-
 5 files changed, 147 insertions(+), 34 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 6e24384..4f0eb17 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -226,11 +226,29 @@ static void backup_handle_dirty_bitmap(BackupBlockJob *job, int ret)
     }
 }
 
+static void backup_txn_commit(BlockJob *job)
+{
+    BackupBlockJob *s = container_of(job, BackupBlockJob, common);
+    if (s->sync_bitmap) {
+        backup_handle_dirty_bitmap(s, 0);
+    }
+}
+
+static void backup_txn_abort(BlockJob *job)
+{
+    BackupBlockJob *s = container_of(job, BackupBlockJob, common);
+    if (s->sync_bitmap) {
+        backup_handle_dirty_bitmap(s, -1);
+    }
+}
+
 static const BlockJobDriver backup_job_driver = {
     .instance_size  = sizeof(BackupBlockJob),
     .job_type       = BLOCK_JOB_TYPE_BACKUP,
     .set_speed      = backup_set_speed,
     .iostatus_reset = backup_iostatus_reset,
+    .txn_commit     = backup_txn_commit,
+    .txn_abort      = backup_txn_abort,
 };
 
 static BlockErrorAction backup_error_action(BackupBlockJob *job,
@@ -444,7 +462,7 @@ static void coroutine_fn backup_run(void *opaque)
     qemu_co_rwlock_wrlock(&job->flush_rwlock);
     qemu_co_rwlock_unlock(&job->flush_rwlock);
 
-    if (job->sync_bitmap) {
+    if (!job->common.txn && job->sync_bitmap) {
         backup_handle_dirty_bitmap(job, ret);
     }
     hbitmap_free(job->bitmap);
@@ -463,7 +481,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;
 
@@ -545,6 +563,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 116e144..30ea52d 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;
@@ -2578,15 +2611,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;
@@ -2703,7 +2738,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);
@@ -2714,19 +2749,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;
@@ -2764,7 +2824,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);
@@ -2773,6 +2833,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

  parent reply	other threads:[~2015-07-10  3:48 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-10  3:46 [Qemu-devel] [PATCH v3 00/15] block: incremental backup transactions using BlockJobTxn Fam Zheng
2015-07-10  3:46 ` [Qemu-devel] [PATCH v3 01/15] qapi: Add transaction support to block-dirty-bitmap operations Fam Zheng
2015-07-10  3:46 ` [Qemu-devel] [PATCH v3 02/15] iotests: add transactional incremental backup test Fam Zheng
2015-07-10  3:46 ` [Qemu-devel] [PATCH v3 03/15] block: rename BlkTransactionState and BdrvActionOps Fam Zheng
2015-07-10  3:46 ` [Qemu-devel] [PATCH v3 04/15] block: keep bitmap if incremental backup job is cancelled Fam Zheng
2015-07-13 19:48   ` John Snow
2015-07-10  3:46 ` [Qemu-devel] [PATCH v3 05/15] backup: Extract dirty bitmap handling as a separate function Fam Zheng
2015-07-13 23:06   ` John Snow
2015-07-14  2:46     ` Fam Zheng
2015-07-14  8:26   ` Stefan Hajnoczi
2015-07-10  3:46 ` [Qemu-devel] [PATCH v3 06/15] blockjob: Add .txn_commit and .txn_abort transaction actions Fam Zheng
2015-07-13 23:06   ` John Snow
2015-07-14  8:35   ` Stefan Hajnoczi
2015-07-14  9:26     ` Fam Zheng
2015-07-10  3:46 ` [Qemu-devel] [PATCH v3 07/15] blockjob: Add "completed" and "ret" in BlockJob Fam Zheng
2015-07-13 23:08   ` John Snow
2015-07-10  3:46 ` [Qemu-devel] [PATCH v3 08/15] blockjob: Simplify block_job_finish_sync Fam Zheng
2015-07-13 23:08   ` John Snow
2015-07-10  3:46 ` [Qemu-devel] [PATCH v3 09/15] blockjob: Move BlockJobDeferToMainLoopData into BlockJob Fam Zheng
2015-07-14 10:03   ` Stefan Hajnoczi
2015-07-14 10:36     ` Fam Zheng
2015-07-10  3:46 ` [Qemu-devel] [PATCH v3 10/15] block: add block job transactions Fam Zheng
2015-07-13 23:12   ` John Snow
2015-07-14  3:04     ` Fam Zheng
2015-07-14 15:05       ` John Snow
2015-07-14 10:27   ` Stefan Hajnoczi
2015-07-10  3:46 ` [Qemu-devel] [PATCH v3 11/15] blockdev: make BlockJobTxn available to qmp 'transaction' Fam Zheng
2015-07-10  3:46 ` Fam Zheng [this message]
2015-07-13 23:14   ` [Qemu-devel] [PATCH v3 12/15] block/backup: support block job transactions John Snow
2015-07-14  3:12     ` Fam Zheng
2015-07-14 10:32   ` Stefan Hajnoczi
2015-07-10  3:46 ` [Qemu-devel] [PATCH v3 13/15] iotests: 124 - transactional failure test Fam Zheng
2015-07-10  3:46 ` [Qemu-devel] [PATCH v3 14/15] qmp-commands.hx: Update the supported 'transaction' operations Fam Zheng
2015-07-10  3:46 ` [Qemu-devel] [PATCH v3 15/15] tests: add BlockJobTxn unit test Fam Zheng
2015-07-13 23:14   ` John Snow

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1436500012-32593-13-git-send-email-famz@redhat.com \
    --to=famz@redhat.com \
    --cc=jcody@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=vsementsov@parallels.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.