[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v4 16/21] backup: Switch block_backup.h to byte-
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-block] [PATCH v4 16/21] backup: Switch block_backup.h to byte-based |
Date: |
Thu, 6 Jul 2017 16:11:51 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 05.07.2017 um 23:08 hat Eric Blake geschrieben:
> We are gradually converting to byte-based interfaces, as they are
> easier to reason about than sector-based. Continue by converting
> the public interface to backup jobs (no semantic change), including
> a change to CowRequest to track by bytes instead of cluster indices.
>
> Note that this does not change the difference between the public
> interface (starting point, and size of the subsequent range) and
> the internal interface (starting and end points).
>
> Signed-off-by: Eric Blake <address@hidden>
> Reviewed-by: John Snow <address@hidden>
> Reviewed-by: Xie Changlong <address@hidden>
> Reviewed-by: Jeff Cody <address@hidden>
> diff --git a/block/backup.c b/block/backup.c
> index 4e64710..cfbd921 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -55,7 +55,7 @@ static inline int64_t cluster_size_sectors(BackupBlockJob
> *job)
>
> /* See if in-flight requests overlap and wait for them to complete */
> static void coroutine_fn wait_for_overlapping_requests(BackupBlockJob *job,
> - int64_t start,
> + int64_t offset,
> int64_t end)
Not sure how useful this rename is. I think I would have renamed either
both or probably actually none of the parameters. start/end is a nice
pair, offset/end is kind of unexpected. start_offset/end_offset would be
an option if you do want to rename, but I don't see a need for it in
this very short function.
> {
> CowRequest *req;
> @@ -64,7 +64,7 @@ static void coroutine_fn
> wait_for_overlapping_requests(BackupBlockJob *job,
> do {
> retry = false;
> QLIST_FOREACH(req, &job->inflight_reqs, list) {
> - if (end > req->start && start < req->end) {
> + if (end > req->start_byte && offset < req->end_byte) {
> qemu_co_queue_wait(&req->wait_queue, NULL);
> retry = true;
> break;
> @@ -75,10 +75,10 @@ static void coroutine_fn
> wait_for_overlapping_requests(BackupBlockJob *job,
>
> /* Keep track of an in-flight request */
> static void cow_request_begin(CowRequest *req, BackupBlockJob *job,
> - int64_t start, int64_t end)
> + int64_t offset, int64_t end)
Same here.
But again, not a correctness issue.
Reviewed-by: Kevin Wolf <address@hidden>
- Re: [Qemu-block] [PATCH v4 11/21] mirror: Switch mirror_cow_align() to byte-based, (continued)
- [Qemu-block] [PATCH v4 13/21] mirror: Switch mirror_iteration() to byte-based, Eric Blake, 2017/07/05
- [Qemu-block] [PATCH v4 14/21] block: Drop unused bdrv_round_sectors_to_clusters(), Eric Blake, 2017/07/05
- [Qemu-block] [PATCH v4 16/21] backup: Switch block_backup.h to byte-based, Eric Blake, 2017/07/05
- Re: [Qemu-block] [PATCH v4 16/21] backup: Switch block_backup.h to byte-based,
Kevin Wolf <=
- [Qemu-block] [PATCH v4 15/21] backup: Switch BackupBlockJob to byte-based, Eric Blake, 2017/07/05
- [Qemu-block] [PATCH v4 17/21] backup: Switch backup_do_cow() to byte-based, Eric Blake, 2017/07/05
- [Qemu-block] [PATCH v4 18/21] backup: Switch backup_run() to byte-based, Eric Blake, 2017/07/05
- [Qemu-block] [PATCH v4 19/21] block: Make bdrv_is_allocated() byte-based, Eric Blake, 2017/07/05