diff mbox

[v2,4/5] block/backup: Pack Notifier within BackupBlockJob

Message ID 1453852499-25800-5-git-send-email-jsnow@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

John Snow Jan. 26, 2016, 11:54 p.m. UTC 
Instead of relying on peeking at bs->job, we want to explicitly get
a reference to the job that was involved in this notifier callback.

Pack the Notifier inside of the BackupBlockJob so we can use
container_of to get a reference back to the BackupBlockJob object.

This cuts out one more case where we rely unnecessarily on bs->job.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
 block/backup.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

Comments

Fam Zheng Jan. 27, 2016, 2:21 a.m. UTC | #1 
On Tue, 01/26 18:54, John Snow wrote:
> Instead of relying on peeking at bs->job, we want to explicitly get
> a reference to the job that was involved in this notifier callback.
> 
> Pack the Notifier inside of the BackupBlockJob so we can use
> container_of to get a reference back to the BackupBlockJob object.
> 
> This cuts out one more case where we rely unnecessarily on bs->job.
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  block/backup.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/block/backup.c b/block/backup.c
> index ce96b45..735cbe6 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -46,6 +46,7 @@ typedef struct BackupBlockJob {
>      CoRwlock flush_rwlock;
>      uint64_t sectors_read;
>      HBitmap *bitmap;
> +    NotifierWithReturn before_write;
>      QLIST_HEAD(, CowRequest) inflight_reqs;
>  } BackupBlockJob;
>  
> @@ -87,11 +88,11 @@ static void cow_request_end(CowRequest *req)
>  }
>  
>  static int coroutine_fn backup_do_cow(BlockDriverState *bs,
> +                                      BackupBlockJob *job,
>                                        int64_t sector_num, int nb_sectors,
>                                        bool *error_is_read,
>                                        bool is_write_notifier)
>  {
> -    BackupBlockJob *job = (BackupBlockJob *)bs->job;
>      CowRequest cow_request;
>      struct iovec iov;
>      QEMUIOVector bounce_qiov;
> @@ -189,6 +190,7 @@ static int coroutine_fn backup_before_write_notify(
>          NotifierWithReturn *notifier,
>          void *opaque)
>  {
> +    BackupBlockJob *job = container_of(notifier, BackupBlockJob, before_write);
>      BdrvTrackedRequest *req = opaque;
>      int64_t sector_num = req->offset >> BDRV_SECTOR_BITS;
>      int nb_sectors = req->bytes >> BDRV_SECTOR_BITS;
> @@ -196,7 +198,8 @@ static int coroutine_fn backup_before_write_notify(
>      assert((req->offset & (BDRV_SECTOR_SIZE - 1)) == 0);
>      assert((req->bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
>  
> -    return backup_do_cow(req->bs, sector_num, nb_sectors, NULL, true);
> +    return backup_do_cow(req->bs, job, sector_num,
> +                         nb_sectors, NULL, true);
>  }
>  
>  static void backup_set_speed(BlockJob *job, int64_t speed, Error **errp)
> @@ -344,7 +347,8 @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
>                  if (yield_and_check(job)) {
>                      return ret;
>                  }
> -                ret = backup_do_cow(bs, cluster * BACKUP_SECTORS_PER_CLUSTER,
> +                ret = backup_do_cow(bs, job,
> +                                    cluster * BACKUP_SECTORS_PER_CLUSTER,
>                                      BACKUP_SECTORS_PER_CLUSTER, &error_is_read,
>                                      false);
>                  if ((ret < 0) &&
> @@ -380,9 +384,6 @@ static void coroutine_fn backup_run(void *opaque)
>      BlockDriverState *bs = job->common.bs;
>      BlockDriverState *target = job->target;
>      BlockdevOnError on_target_error = job->on_target_error;
> -    NotifierWithReturn before_write = {
> -        .notify = backup_before_write_notify,
> -    };
>      int64_t start, end;
>      int ret = 0;
>  
> @@ -400,7 +401,8 @@ static void coroutine_fn backup_run(void *opaque)
>          blk_iostatus_enable(target->blk);
>      }
>  
> -    bdrv_add_before_write_notifier(bs, &before_write);
> +    job->before_write.notify = backup_before_write_notify;
> +    bdrv_add_before_write_notifier(bs, &job->before_write);
>  
>      if (job->sync_mode == MIRROR_SYNC_MODE_NONE) {
>          while (!block_job_is_cancelled(&job->common)) {
> @@ -452,7 +454,7 @@ static void coroutine_fn backup_run(void *opaque)
>                  }
>              }
>              /* FULL sync mode we copy the whole drive. */
> -            ret = backup_do_cow(bs, start * BACKUP_SECTORS_PER_CLUSTER,
> +            ret = backup_do_cow(bs, job, start * BACKUP_SECTORS_PER_CLUSTER,
>                      BACKUP_SECTORS_PER_CLUSTER, &error_is_read, false);
>              if (ret < 0) {
>                  /* Depending on error action, fail now or retry cluster */
> @@ -468,7 +470,7 @@ static void coroutine_fn backup_run(void *opaque)
>          }
>      }
>  
> -    notifier_with_return_remove(&before_write);
> +    notifier_with_return_remove(&job->before_write);
>  
>      /* wait until pending backup_do_cow() calls have completed */
>      qemu_co_rwlock_wrlock(&job->flush_rwlock);
> -- 
> 2.4.3
> 
> 

Reviewed-by: Fam Zheng <famz@redhat.com>
diff mbox

Patch

diff --git a/block/backup.c b/block/backup.c
index ce96b45..735cbe6 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -46,6 +46,7 @@  typedef struct BackupBlockJob {
     CoRwlock flush_rwlock;
     uint64_t sectors_read;
     HBitmap *bitmap;
+    NotifierWithReturn before_write;
     QLIST_HEAD(, CowRequest) inflight_reqs;
 } BackupBlockJob;
 
@@ -87,11 +88,11 @@  static void cow_request_end(CowRequest *req)
 }
 
 static int coroutine_fn backup_do_cow(BlockDriverState *bs,
+                                      BackupBlockJob *job,
                                       int64_t sector_num, int nb_sectors,
                                       bool *error_is_read,
                                       bool is_write_notifier)
 {
-    BackupBlockJob *job = (BackupBlockJob *)bs->job;
     CowRequest cow_request;
     struct iovec iov;
     QEMUIOVector bounce_qiov;
@@ -189,6 +190,7 @@  static int coroutine_fn backup_before_write_notify(
         NotifierWithReturn *notifier,
         void *opaque)
 {
+    BackupBlockJob *job = container_of(notifier, BackupBlockJob, before_write);
     BdrvTrackedRequest *req = opaque;
     int64_t sector_num = req->offset >> BDRV_SECTOR_BITS;
     int nb_sectors = req->bytes >> BDRV_SECTOR_BITS;
@@ -196,7 +198,8 @@  static int coroutine_fn backup_before_write_notify(
     assert((req->offset & (BDRV_SECTOR_SIZE - 1)) == 0);
     assert((req->bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
 
-    return backup_do_cow(req->bs, sector_num, nb_sectors, NULL, true);
+    return backup_do_cow(req->bs, job, sector_num,
+                         nb_sectors, NULL, true);
 }
 
 static void backup_set_speed(BlockJob *job, int64_t speed, Error **errp)
@@ -344,7 +347,8 @@  static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
                 if (yield_and_check(job)) {
                     return ret;
                 }
-                ret = backup_do_cow(bs, cluster * BACKUP_SECTORS_PER_CLUSTER,
+                ret = backup_do_cow(bs, job,
+                                    cluster * BACKUP_SECTORS_PER_CLUSTER,
                                     BACKUP_SECTORS_PER_CLUSTER, &error_is_read,
                                     false);
                 if ((ret < 0) &&
@@ -380,9 +384,6 @@  static void coroutine_fn backup_run(void *opaque)
     BlockDriverState *bs = job->common.bs;
     BlockDriverState *target = job->target;
     BlockdevOnError on_target_error = job->on_target_error;
-    NotifierWithReturn before_write = {
-        .notify = backup_before_write_notify,
-    };
     int64_t start, end;
     int ret = 0;
 
@@ -400,7 +401,8 @@  static void coroutine_fn backup_run(void *opaque)
         blk_iostatus_enable(target->blk);
     }
 
-    bdrv_add_before_write_notifier(bs, &before_write);
+    job->before_write.notify = backup_before_write_notify;
+    bdrv_add_before_write_notifier(bs, &job->before_write);
 
     if (job->sync_mode == MIRROR_SYNC_MODE_NONE) {
         while (!block_job_is_cancelled(&job->common)) {
@@ -452,7 +454,7 @@  static void coroutine_fn backup_run(void *opaque)
                 }
             }
             /* FULL sync mode we copy the whole drive. */
-            ret = backup_do_cow(bs, start * BACKUP_SECTORS_PER_CLUSTER,
+            ret = backup_do_cow(bs, job, start * BACKUP_SECTORS_PER_CLUSTER,
                     BACKUP_SECTORS_PER_CLUSTER, &error_is_read, false);
             if (ret < 0) {
                 /* Depending on error action, fail now or retry cluster */
@@ -468,7 +470,7 @@  static void coroutine_fn backup_run(void *opaque)
         }
     }
 
-    notifier_with_return_remove(&before_write);
+    notifier_with_return_remove(&job->before_write);
 
     /* wait until pending backup_do_cow() calls have completed */
     qemu_co_rwlock_wrlock(&job->flush_rwlock);