diff mbox

[v2,07/16] backup: Do initial aio context move of target via BB interface

Message ID 20170419094356.19826-8-famz@redhat.com
State New
Headers show

Commit Message

Fam Zheng April 19, 2017, 9:43 a.m. UTC 
The old aio context check is hacky because when it was added we didn't
have the permission system to enforce a general rule. It only checks if
the target BDS has a BB, which is in fact insufficient because there may
be other BBs in the graph that cannot handle the aio context change.

Do this through blk_set_aio_context interface, in backup_job_create()
which is a central place for both drive-backup and blockdev-backup, to
take care of the compatibility check.

Also the bdrv_set_aio_context in do_drive_backup could have been
conditional, to save a recursion when possible.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/backup.c |  9 +++++++++
 blockdev.c     | 14 --------------
 2 files changed, 9 insertions(+), 14 deletions(-)

Comments

Stefan Hajnoczi May 11, 2017, 8 p.m. UTC | #1 
On Wed, Apr 19, 2017 at 05:43:47PM +0800, Fam Zheng wrote:
> The old aio context check is hacky because when it was added we didn't
> have the permission system to enforce a general rule. It only checks if
> the target BDS has a BB, which is in fact insufficient because there may
> be other BBs in the graph that cannot handle the aio context change.
> 
> Do this through blk_set_aio_context interface, in backup_job_create()
> which is a central place for both drive-backup and blockdev-backup, to
> take care of the compatibility check.
> 
> Also the bdrv_set_aio_context in do_drive_backup could have been
> conditional, to save a recursion when possible.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block/backup.c |  9 +++++++++
>  blockdev.c     | 14 --------------
>  2 files changed, 9 insertions(+), 14 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff mbox

Patch

diff --git a/block/backup.c b/block/backup.c
index 546c5c5..9136f91 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -564,6 +564,7 @@  BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
     BlockDriverInfo bdi;
     BackupBlockJob *job = NULL;
     int ret;
+    AioContext *aio_context, *target_context;
 
     assert(bs);
     assert(target);
@@ -644,6 +645,14 @@  BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
         goto error;
     }
 
+    aio_context = bdrv_get_aio_context(bs);
+    target_context = bdrv_get_aio_context(target);
+    if (target_context != aio_context) {
+        aio_context_acquire(target_context);
+        blk_set_aio_context(job->target, aio_context);
+        aio_context_release(target_context);
+    }
+
     job->on_source_error = on_source_error;
     job->on_target_error = on_target_error;
     job->sync_mode = sync_mode;
diff --git a/blockdev.c b/blockdev.c
index 4927914..dfd1385 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3259,8 +3259,6 @@  static BlockJob *do_drive_backup(DriveBackup *backup, BlockJobTxn *txn,
         goto out;
     }
 
-    bdrv_set_aio_context(target_bs, aio_context);
-
     if (backup->has_bitmap) {
         bmap = bdrv_find_dirty_bitmap(bs, backup->bitmap);
         if (!bmap) {
@@ -3337,18 +3335,6 @@  BlockJob *do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn,
     if (!target_bs) {
         goto out;
     }
-
-    if (bdrv_get_aio_context(target_bs) != aio_context) {
-        if (!bdrv_has_blk(target_bs)) {
-            /* The target BDS is not attached, we can safely move it to another
-             * AioContext. */
-            bdrv_set_aio_context(target_bs, aio_context);
-        } else {
-            error_setg(errp, "Target is attached to a different thread from "
-                             "source.");
-            goto out;
-        }
-    }
     job = backup_job_create(backup->job_id, bs, target_bs, backup->speed,
                             backup->sync, NULL, backup->compress,
                             backup->on_source_error, backup->on_target_error,