Message ID | 20170411222945.11741-14-eblake@redhat.com |
---|---|
State | New |
Headers | show |
On 04/11/2017 06:29 PM, Eric Blake wrote: > 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. > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > include/block/block_backup.h | 11 +++++------ > block/backup.c | 29 ++++++++++++++--------------- > block/replication.c | 12 ++++++++---- > 3 files changed, 27 insertions(+), 25 deletions(-) > > diff --git a/include/block/block_backup.h b/include/block/block_backup.h > index 8a75947..994a3bd 100644 > --- a/include/block/block_backup.h > +++ b/include/block/block_backup.h > @@ -21,17 +21,16 @@ > #include "block/block_int.h" > > typedef struct CowRequest { > - int64_t start; > - int64_t end; > + int64_t start_byte; > + int64_t end_byte; > QLIST_ENTRY(CowRequest) list; > CoQueue wait_queue; /* coroutines blocked on this request */ > } CowRequest; > > -void backup_wait_for_overlapping_requests(BlockJob *job, int64_t sector_num, > - int nb_sectors); > +void backup_wait_for_overlapping_requests(BlockJob *job, int64_t offset, > + uint64_t bytes); > void backup_cow_request_begin(CowRequest *req, BlockJob *job, > - int64_t sector_num, > - int nb_sectors); > + int64_t offset, uint64_t bytes); > void backup_cow_request_end(CowRequest *req); Should we adjust the parameter names of cow_request_begin and wait_for_overlapping_requests, too? > > void backup_do_checkpoint(BlockJob *job, Error **errp); > diff --git a/block/backup.c b/block/backup.c > index a64a162..0502c1a 100644 > --- a/block/backup.c > +++ b/block/backup.c > @@ -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 && start < req->end_byte) { > qemu_co_queue_wait(&req->wait_queue, NULL); > retry = true; > break; > @@ -77,8 +77,8 @@ static void coroutine_fn wait_for_overlapping_requests(BackupBlockJob *job, > static void cow_request_begin(CowRequest *req, BackupBlockJob *job, > int64_t start, int64_t end) > { > - req->start = start; > - req->end = end; > + req->start_byte = start; > + req->end_byte = end; > qemu_co_queue_init(&req->wait_queue); > QLIST_INSERT_HEAD(&job->inflight_reqs, req, list); > } > @@ -114,8 +114,10 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job, > sector_num * BDRV_SECTOR_SIZE, > nb_sectors * BDRV_SECTOR_SIZE); > > - wait_for_overlapping_requests(job, start, end); > - cow_request_begin(&cow_request, job, start, end); > + wait_for_overlapping_requests(job, start * job->cluster_size, > + end * job->cluster_size); > + cow_request_begin(&cow_request, job, start * job->cluster_size, > + end * job->cluster_size); > > for (; start < end; start++) { > if (test_bit(start, job->done_bitmap)) { > @@ -277,32 +279,29 @@ void backup_do_checkpoint(BlockJob *job, Error **errp) > bitmap_zero(backup_job->done_bitmap, len); > } > > -void backup_wait_for_overlapping_requests(BlockJob *job, int64_t sector_num, > - int nb_sectors) > +void backup_wait_for_overlapping_requests(BlockJob *job, int64_t offset, > + uint64_t bytes) > { > BackupBlockJob *backup_job = container_of(job, BackupBlockJob, common); > - int64_t sectors_per_cluster = cluster_size_sectors(backup_job); > int64_t start, end; > > assert(job->driver->job_type == BLOCK_JOB_TYPE_BACKUP); > > - start = sector_num / sectors_per_cluster; > - end = DIV_ROUND_UP(sector_num + nb_sectors, sectors_per_cluster); > + start = QEMU_ALIGN_DOWN(offset, backup_job->cluster_size); > + end = QEMU_ALIGN_UP(offset + bytes, backup_job->cluster_size); > wait_for_overlapping_requests(backup_job, start, end); > } > > void backup_cow_request_begin(CowRequest *req, BlockJob *job, > - int64_t sector_num, > - int nb_sectors) > + int64_t offset, uint64_t bytes) > { > BackupBlockJob *backup_job = container_of(job, BackupBlockJob, common); > - int64_t sectors_per_cluster = cluster_size_sectors(backup_job); > int64_t start, end; > > assert(job->driver->job_type == BLOCK_JOB_TYPE_BACKUP); > > - start = sector_num / sectors_per_cluster; > - end = DIV_ROUND_UP(sector_num + nb_sectors, sectors_per_cluster); > + start = QEMU_ALIGN_DOWN(offset, backup_job->cluster_size); > + end = QEMU_ALIGN_UP(offset + bytes, backup_job->cluster_size); > cow_request_begin(req, backup_job, start, end); > } > > diff --git a/block/replication.c b/block/replication.c > index bf3c395..414ecc4 100644 > --- a/block/replication.c > +++ b/block/replication.c > @@ -234,10 +234,14 @@ static coroutine_fn int replication_co_readv(BlockDriverState *bs, > } > > if (job) { > - backup_wait_for_overlapping_requests(child->bs->job, sector_num, > - remaining_sectors); > - backup_cow_request_begin(&req, child->bs->job, sector_num, > - remaining_sectors); > + uint64_t remaining_bytes = remaining_sectors * BDRV_SECTOR_SIZE; > + > + backup_wait_for_overlapping_requests(child->bs->job, > + sector_num * BDRV_SECTOR_SIZE, > + remaining_bytes); > + backup_cow_request_begin(&req, child->bs->job, > + sector_num * BDRV_SECTOR_SIZE, > + remaining_bytes); > ret = bdrv_co_readv(bs->file, sector_num, remaining_sectors, > qiov); > backup_cow_request_end(&req); >
On 04/17/2017 06:24 PM, John Snow wrote: > > > On 04/11/2017 06:29 PM, Eric Blake wrote: >> 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. >> >> Signed-off-by: Eric Blake <eblake@redhat.com> >> --- >> >> -void backup_wait_for_overlapping_requests(BlockJob *job, int64_t sector_num, >> - int nb_sectors); >> +void backup_wait_for_overlapping_requests(BlockJob *job, int64_t offset, >> + uint64_t bytes); >> void backup_cow_request_begin(CowRequest *req, BlockJob *job, >> - int64_t sector_num, >> - int nb_sectors); >> + int64_t offset, uint64_t bytes); >> void backup_cow_request_end(CowRequest *req); > > Should we adjust the parameter names of cow_request_begin and > wait_for_overlapping_requests, too? Sure, I can do that.
diff --git a/include/block/block_backup.h b/include/block/block_backup.h index 8a75947..994a3bd 100644 --- a/include/block/block_backup.h +++ b/include/block/block_backup.h @@ -21,17 +21,16 @@ #include "block/block_int.h" typedef struct CowRequest { - int64_t start; - int64_t end; + int64_t start_byte; + int64_t end_byte; QLIST_ENTRY(CowRequest) list; CoQueue wait_queue; /* coroutines blocked on this request */ } CowRequest; -void backup_wait_for_overlapping_requests(BlockJob *job, int64_t sector_num, - int nb_sectors); +void backup_wait_for_overlapping_requests(BlockJob *job, int64_t offset, + uint64_t bytes); void backup_cow_request_begin(CowRequest *req, BlockJob *job, - int64_t sector_num, - int nb_sectors); + int64_t offset, uint64_t bytes); void backup_cow_request_end(CowRequest *req); void backup_do_checkpoint(BlockJob *job, Error **errp); diff --git a/block/backup.c b/block/backup.c index a64a162..0502c1a 100644 --- a/block/backup.c +++ b/block/backup.c @@ -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 && start < req->end_byte) { qemu_co_queue_wait(&req->wait_queue, NULL); retry = true; break; @@ -77,8 +77,8 @@ static void coroutine_fn wait_for_overlapping_requests(BackupBlockJob *job, static void cow_request_begin(CowRequest *req, BackupBlockJob *job, int64_t start, int64_t end) { - req->start = start; - req->end = end; + req->start_byte = start; + req->end_byte = end; qemu_co_queue_init(&req->wait_queue); QLIST_INSERT_HEAD(&job->inflight_reqs, req, list); } @@ -114,8 +114,10 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job, sector_num * BDRV_SECTOR_SIZE, nb_sectors * BDRV_SECTOR_SIZE); - wait_for_overlapping_requests(job, start, end); - cow_request_begin(&cow_request, job, start, end); + wait_for_overlapping_requests(job, start * job->cluster_size, + end * job->cluster_size); + cow_request_begin(&cow_request, job, start * job->cluster_size, + end * job->cluster_size); for (; start < end; start++) { if (test_bit(start, job->done_bitmap)) { @@ -277,32 +279,29 @@ void backup_do_checkpoint(BlockJob *job, Error **errp) bitmap_zero(backup_job->done_bitmap, len); } -void backup_wait_for_overlapping_requests(BlockJob *job, int64_t sector_num, - int nb_sectors) +void backup_wait_for_overlapping_requests(BlockJob *job, int64_t offset, + uint64_t bytes) { BackupBlockJob *backup_job = container_of(job, BackupBlockJob, common); - int64_t sectors_per_cluster = cluster_size_sectors(backup_job); int64_t start, end; assert(job->driver->job_type == BLOCK_JOB_TYPE_BACKUP); - start = sector_num / sectors_per_cluster; - end = DIV_ROUND_UP(sector_num + nb_sectors, sectors_per_cluster); + start = QEMU_ALIGN_DOWN(offset, backup_job->cluster_size); + end = QEMU_ALIGN_UP(offset + bytes, backup_job->cluster_size); wait_for_overlapping_requests(backup_job, start, end); } void backup_cow_request_begin(CowRequest *req, BlockJob *job, - int64_t sector_num, - int nb_sectors) + int64_t offset, uint64_t bytes) { BackupBlockJob *backup_job = container_of(job, BackupBlockJob, common); - int64_t sectors_per_cluster = cluster_size_sectors(backup_job); int64_t start, end; assert(job->driver->job_type == BLOCK_JOB_TYPE_BACKUP); - start = sector_num / sectors_per_cluster; - end = DIV_ROUND_UP(sector_num + nb_sectors, sectors_per_cluster); + start = QEMU_ALIGN_DOWN(offset, backup_job->cluster_size); + end = QEMU_ALIGN_UP(offset + bytes, backup_job->cluster_size); cow_request_begin(req, backup_job, start, end); } diff --git a/block/replication.c b/block/replication.c index bf3c395..414ecc4 100644 --- a/block/replication.c +++ b/block/replication.c @@ -234,10 +234,14 @@ static coroutine_fn int replication_co_readv(BlockDriverState *bs, } if (job) { - backup_wait_for_overlapping_requests(child->bs->job, sector_num, - remaining_sectors); - backup_cow_request_begin(&req, child->bs->job, sector_num, - remaining_sectors); + uint64_t remaining_bytes = remaining_sectors * BDRV_SECTOR_SIZE; + + backup_wait_for_overlapping_requests(child->bs->job, + sector_num * BDRV_SECTOR_SIZE, + remaining_bytes); + backup_cow_request_begin(&req, child->bs->job, + sector_num * BDRV_SECTOR_SIZE, + remaining_bytes); ret = bdrv_co_readv(bs->file, sector_num, remaining_sectors, qiov); backup_cow_request_end(&req);
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. Signed-off-by: Eric Blake <eblake@redhat.com> --- include/block/block_backup.h | 11 +++++------ block/backup.c | 29 ++++++++++++++--------------- block/replication.c | 12 ++++++++---- 3 files changed, 27 insertions(+), 25 deletions(-)