[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 4/5] block/backup: Pack Notifier within Backu
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH v2 4/5] block/backup: Pack Notifier within BackupBlockJob |
Date: |
Wed, 27 Jan 2016 10:21:41 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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 <address@hidden>
> Signed-off-by: John Snow <address@hidden>
> ---
> 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 <address@hidden>
- [Qemu-devel] [PATCH v2 0/5] block: reduce reliance on bs->job pointer, John Snow, 2016/01/26
- [Qemu-devel] [PATCH v2 2/5] block: Allow stream_start to return job references, John Snow, 2016/01/26
- [Qemu-devel] [PATCH v2 1/5] block: Allow mirror_start to return job references, John Snow, 2016/01/26
- [Qemu-devel] [PATCH v2 3/5] block: allow backup_start to return job references, John Snow, 2016/01/26
- [Qemu-devel] [PATCH v2 4/5] block/backup: Pack Notifier within BackupBlockJob, John Snow, 2016/01/26
- Re: [Qemu-devel] [PATCH v2 4/5] block/backup: Pack Notifier within BackupBlockJob,
Fam Zheng <=
- [Qemu-devel] [PATCH v2 5/5] blockjob: add Job parameter to BlockCompletionFunc, John Snow, 2016/01/26