From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46945) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dR3Ir-0005Wt-Ul for qemu-devel@nongnu.org; Fri, 30 Jun 2017 17:18:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dR3Iq-0000e6-PH for qemu-devel@nongnu.org; Fri, 30 Jun 2017 17:18:13 -0400 Date: Fri, 30 Jun 2017 17:18:01 -0400 From: Jeff Cody Message-ID: <20170630211801.GM4997@localhost.localdomain> References: <20170627192458.15519-1-eblake@redhat.com> <20170627192458.15519-15-eblake@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170627192458.15519-15-eblake@redhat.com> Subject: Re: [Qemu-devel] [PATCH v3 14/20] backup: Switch BackupBlockJob to byte-based List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org, kwolf@redhat.com, qemu-block@nongnu.org, jsnow@redhat.com, Max Reitz On Tue, Jun 27, 2017 at 02:24:52PM -0500, Eric Blake wrote: > We are gradually converting to byte-based interfaces, as they are > easier to reason about than sector-based. Continue by converting an > internal structure (no semantic change), and all references to > tracking progress. Drop a redundant local variable bytes_per_cluster. > > Signed-off-by: Eric Blake > Reviewed-by: John Snow > Reviewed-by: Jeff Cody > --- > v2: no change > --- > block/backup.c | 33 +++++++++++++++------------------ > 1 file changed, 15 insertions(+), 18 deletions(-) > > diff --git a/block/backup.c b/block/backup.c > index 06431ac..4e64710 100644 > --- a/block/backup.c > +++ b/block/backup.c > @@ -39,7 +39,7 @@ typedef struct BackupBlockJob { > BlockdevOnError on_source_error; > BlockdevOnError on_target_error; > CoRwlock flush_rwlock; > - uint64_t sectors_read; > + uint64_t bytes_read; > unsigned long *done_bitmap; > int64_t cluster_size; > bool compress; > @@ -102,16 +102,15 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job, > void *bounce_buffer = NULL; > int ret = 0; > int64_t sectors_per_cluster = cluster_size_sectors(job); > - int64_t bytes_per_cluster = sectors_per_cluster * BDRV_SECTOR_SIZE; > - int64_t start, end; > - int n; > + int64_t start, end; /* clusters */ > + int n; /* bytes */ > > qemu_co_rwlock_rdlock(&job->flush_rwlock); > > start = sector_num / sectors_per_cluster; > end = DIV_ROUND_UP(sector_num + nb_sectors, sectors_per_cluster); > > - trace_backup_do_cow_enter(job, start * bytes_per_cluster, > + trace_backup_do_cow_enter(job, start * job->cluster_size, > sector_num * BDRV_SECTOR_SIZE, > nb_sectors * BDRV_SECTOR_SIZE); > > @@ -120,28 +119,27 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job, > > for (; start < end; start++) { > if (test_bit(start, job->done_bitmap)) { > - trace_backup_do_cow_skip(job, start * bytes_per_cluster); > + trace_backup_do_cow_skip(job, start * job->cluster_size); > continue; /* already copied */ > } > > - trace_backup_do_cow_process(job, start * bytes_per_cluster); > + trace_backup_do_cow_process(job, start * job->cluster_size); > > - n = MIN(sectors_per_cluster, > - job->common.len / BDRV_SECTOR_SIZE - > - start * sectors_per_cluster); > + n = MIN(job->cluster_size, > + job->common.len - start * job->cluster_size); > > if (!bounce_buffer) { > bounce_buffer = blk_blockalign(blk, job->cluster_size); > } > iov.iov_base = bounce_buffer; > - iov.iov_len = n * BDRV_SECTOR_SIZE; > + iov.iov_len = n; > qemu_iovec_init_external(&bounce_qiov, &iov, 1); > > ret = blk_co_preadv(blk, start * job->cluster_size, > bounce_qiov.size, &bounce_qiov, > is_write_notifier ? BDRV_REQ_NO_SERIALISING : 0); > if (ret < 0) { > - trace_backup_do_cow_read_fail(job, start * bytes_per_cluster, ret); > + trace_backup_do_cow_read_fail(job, start * job->cluster_size, ret); > if (error_is_read) { > *error_is_read = true; > } > @@ -157,7 +155,7 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job, > job->compress ? BDRV_REQ_WRITE_COMPRESSED : 0); > } > if (ret < 0) { > - trace_backup_do_cow_write_fail(job, start * bytes_per_cluster, ret); > + trace_backup_do_cow_write_fail(job, start * job->cluster_size, ret); > if (error_is_read) { > *error_is_read = false; > } > @@ -169,8 +167,8 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job, > /* Publish progress, guest I/O counts as progress too. Note that the > * offset field is an opaque progress value, it is not a disk offset. > */ > - job->sectors_read += n; > - job->common.offset += n * BDRV_SECTOR_SIZE; > + job->bytes_read += n; > + job->common.offset += n; > } > > out: > @@ -363,9 +361,8 @@ static bool coroutine_fn yield_and_check(BackupBlockJob *job) > */ > if (job->common.speed) { > uint64_t delay_ns = ratelimit_calculate_delay(&job->limit, > - job->sectors_read * > - BDRV_SECTOR_SIZE); > - job->sectors_read = 0; > + job->bytes_read); > + job->bytes_read = 0; > block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, delay_ns); > } else { > block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, 0); > -- > 2.9.4 >