Re: [Qemu-block] [PATCH v3 10/20] mirror: Switch mirror_cow_align() to byte-based
On Tue, Jun 27, 2017 at 02:24:48PM -0500, Eric Blake wrote: > We are gradually converting to byte-based interfaces, as they are > easier to reason about than sector-based. Convert another internal > function (no semantic change), and add mirror_clip_bytes() as a > counterpart to mirror_clip_sectors(). Some of the conversion is > a bit tricky, requiring temporaries to convert between units; it > will be cleared up in a following patch. > > Signed-off-by: Eric Blake > Reviewed-by: John Snow > Reviewed-by: Jeff Cody > --- > v2: tweak mirror_clip_bytes() signature to match previous patch > --- > block/mirror.c | 64 > ++ > 1 file changed, 38 insertions(+), 26 deletions(-) > > diff --git a/block/mirror.c b/block/mirror.c > index 1a43304..1a4602a 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -176,6 +176,15 @@ static void mirror_read_complete(void *opaque, int ret) > aio_context_release(blk_get_aio_context(s->common.blk)); > } > > +/* Clip bytes relative to offset to not exceed end-of-file */ > +static inline int64_t mirror_clip_bytes(MirrorBlockJob *s, > +int64_t offset, > +int64_t bytes) > +{ > +return MIN(bytes, s->bdev_length - offset); > +} > + > +/* Clip nb_sectors relative to sector_num to not exceed end-of-file */ > static inline int mirror_clip_sectors(MirrorBlockJob *s, >int64_t sector_num, >int nb_sectors) > @@ -184,44 +193,39 @@ static inline int mirror_clip_sectors(MirrorBlockJob *s, > s->bdev_length / BDRV_SECTOR_SIZE - sector_num); > } > > -/* Round sector_num and/or nb_sectors to target cluster if COW is needed, and > - * return the offset of the adjusted tail sector against original. */ > -static int mirror_cow_align(MirrorBlockJob *s, > -int64_t *sector_num, > -int *nb_sectors) > +/* Round offset and/or bytes to target cluster if COW is needed, and > + * return the offset of the adjusted tail against original. */ > +static int mirror_cow_align(MirrorBlockJob *s, int64_t *offset, > +unsigned int *bytes) > { > bool need_cow; > int ret = 0; > -int chunk_sectors = s->granularity >> BDRV_SECTOR_BITS; > -int64_t align_sector_num = *sector_num; > -int align_nb_sectors = *nb_sectors; > -int max_sectors = chunk_sectors * s->max_iov; > +int64_t align_offset = *offset; > +unsigned int align_bytes = *bytes; > +int max_bytes = s->granularity * s->max_iov; > > -need_cow = !test_bit(*sector_num / chunk_sectors, s->cow_bitmap); > -need_cow |= !test_bit((*sector_num + *nb_sectors - 1) / chunk_sectors, > +need_cow = !test_bit(*offset / s->granularity, s->cow_bitmap); > +need_cow |= !test_bit((*offset + *bytes - 1) / s->granularity, >s->cow_bitmap); > if (need_cow) { > -bdrv_round_sectors_to_clusters(blk_bs(s->target), *sector_num, > - *nb_sectors, &align_sector_num, > - &align_nb_sectors); > +bdrv_round_to_clusters(blk_bs(s->target), *offset, *bytes, > + &align_offset, &align_bytes); > } > > -if (align_nb_sectors > max_sectors) { > -align_nb_sectors = max_sectors; > +if (align_bytes > max_bytes) { > +align_bytes = max_bytes; > if (need_cow) { > -align_nb_sectors = QEMU_ALIGN_DOWN(align_nb_sectors, > - s->target_cluster_size >> > - BDRV_SECTOR_BITS); > +align_bytes = QEMU_ALIGN_DOWN(align_bytes, > + s->target_cluster_size); > } > } > -/* Clipping may result in align_nb_sectors unaligned to chunk boundary, > but > +/* Clipping may result in align_bytes unaligned to chunk boundary, but > * that doesn't matter because it's already the end of source image. */ > -align_nb_sectors = mirror_clip_sectors(s, align_sector_num, > - align_nb_sectors); > +align_bytes = mirror_clip_bytes(s, align_offset, align_bytes); > > -ret = align_sector_num + align_nb_sectors - (*sector_num + *nb_sectors); > -*sector_num = align_sector_num; > -*nb_sectors = align_nb_sectors; > +ret = align_offset + align_bytes - (*offset + *bytes); > +*offset = align_offset; > +*bytes = align_bytes; > assert(ret >= 0); > return ret; > } > @@ -257,10 +261,18 @@ static int mirror_do_read(MirrorBlockJob *s, int64_t > sector_num, > nb_sectors = MIN(s->buf_size >> BDRV_SECTOR_BITS, nb_sectors); > nb_sectors = MIN(max_sectors, nb_sectors); > assert(nb_sectors); > +assert(nb_sectors <
[Qemu-block] [PATCH v3 10/20] mirror: Switch mirror_cow_align() to byte-based
We are gradually converting to byte-based interfaces, as they are easier to reason about than sector-based. Convert another internal function (no semantic change), and add mirror_clip_bytes() as a counterpart to mirror_clip_sectors(). Some of the conversion is a bit tricky, requiring temporaries to convert between units; it will be cleared up in a following patch. Signed-off-by: Eric Blake Reviewed-by: John Snow --- v2: tweak mirror_clip_bytes() signature to match previous patch --- block/mirror.c | 64 ++ 1 file changed, 38 insertions(+), 26 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index 1a43304..1a4602a 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -176,6 +176,15 @@ static void mirror_read_complete(void *opaque, int ret) aio_context_release(blk_get_aio_context(s->common.blk)); } +/* Clip bytes relative to offset to not exceed end-of-file */ +static inline int64_t mirror_clip_bytes(MirrorBlockJob *s, +int64_t offset, +int64_t bytes) +{ +return MIN(bytes, s->bdev_length - offset); +} + +/* Clip nb_sectors relative to sector_num to not exceed end-of-file */ static inline int mirror_clip_sectors(MirrorBlockJob *s, int64_t sector_num, int nb_sectors) @@ -184,44 +193,39 @@ static inline int mirror_clip_sectors(MirrorBlockJob *s, s->bdev_length / BDRV_SECTOR_SIZE - sector_num); } -/* Round sector_num and/or nb_sectors to target cluster if COW is needed, and - * return the offset of the adjusted tail sector against original. */ -static int mirror_cow_align(MirrorBlockJob *s, -int64_t *sector_num, -int *nb_sectors) +/* Round offset and/or bytes to target cluster if COW is needed, and + * return the offset of the adjusted tail against original. */ +static int mirror_cow_align(MirrorBlockJob *s, int64_t *offset, +unsigned int *bytes) { bool need_cow; int ret = 0; -int chunk_sectors = s->granularity >> BDRV_SECTOR_BITS; -int64_t align_sector_num = *sector_num; -int align_nb_sectors = *nb_sectors; -int max_sectors = chunk_sectors * s->max_iov; +int64_t align_offset = *offset; +unsigned int align_bytes = *bytes; +int max_bytes = s->granularity * s->max_iov; -need_cow = !test_bit(*sector_num / chunk_sectors, s->cow_bitmap); -need_cow |= !test_bit((*sector_num + *nb_sectors - 1) / chunk_sectors, +need_cow = !test_bit(*offset / s->granularity, s->cow_bitmap); +need_cow |= !test_bit((*offset + *bytes - 1) / s->granularity, s->cow_bitmap); if (need_cow) { -bdrv_round_sectors_to_clusters(blk_bs(s->target), *sector_num, - *nb_sectors, &align_sector_num, - &align_nb_sectors); +bdrv_round_to_clusters(blk_bs(s->target), *offset, *bytes, + &align_offset, &align_bytes); } -if (align_nb_sectors > max_sectors) { -align_nb_sectors = max_sectors; +if (align_bytes > max_bytes) { +align_bytes = max_bytes; if (need_cow) { -align_nb_sectors = QEMU_ALIGN_DOWN(align_nb_sectors, - s->target_cluster_size >> - BDRV_SECTOR_BITS); +align_bytes = QEMU_ALIGN_DOWN(align_bytes, + s->target_cluster_size); } } -/* Clipping may result in align_nb_sectors unaligned to chunk boundary, but +/* Clipping may result in align_bytes unaligned to chunk boundary, but * that doesn't matter because it's already the end of source image. */ -align_nb_sectors = mirror_clip_sectors(s, align_sector_num, - align_nb_sectors); +align_bytes = mirror_clip_bytes(s, align_offset, align_bytes); -ret = align_sector_num + align_nb_sectors - (*sector_num + *nb_sectors); -*sector_num = align_sector_num; -*nb_sectors = align_nb_sectors; +ret = align_offset + align_bytes - (*offset + *bytes); +*offset = align_offset; +*bytes = align_bytes; assert(ret >= 0); return ret; } @@ -257,10 +261,18 @@ static int mirror_do_read(MirrorBlockJob *s, int64_t sector_num, nb_sectors = MIN(s->buf_size >> BDRV_SECTOR_BITS, nb_sectors); nb_sectors = MIN(max_sectors, nb_sectors); assert(nb_sectors); +assert(nb_sectors < BDRV_REQUEST_MAX_SECTORS); ret = nb_sectors; if (s->cow_bitmap) { -ret += mirror_cow_align(s, §or_num, &nb_sectors); +int64_t offset = sector_num * BDRV_SECTOR_SIZE; +unsigned int bytes = nb_sectors * BDRV_SECTOR_SIZE; +int gap; + +gap = mirror_cow_align