On Sat, 02/06 14:24, Max Reitz wrote:
> On 05.02.2016 03:00, Fam Zheng wrote:
> > The "pnum < nb_sectors" condition in deciding whether to actually copy
> > data is unnecessarily strict, and the qiov initialization is
> > unnecessarily for bdrv_aio_write_zeroes and bdrv_aio_discard.
> >
> > Rewrite mirror_iteration to fix both flaws.
> >
> > The output of iotests 109 is updated because we now report the offset
> > and len slightly differently in mirroring progress.
> >
> > Signed-off-by: Fam Zheng <f...@redhat.com>
> > ---
> > block/mirror.c | 335
> > +++++++++++++++++++++++++++------------------
> > tests/qemu-iotests/109.out | 80 +++++------
> > trace-events | 1 -
> > 3 files changed, 243 insertions(+), 173 deletions(-)
> >
> > diff --git a/block/mirror.c b/block/mirror.c
> > index 2c0edfa..48cd0b3 100644
> > --- a/block/mirror.c
> > +++ b/block/mirror.c
>
> [...]
>
> > @@ -449,16 +520,16 @@ static void coroutine_fn mirror_run(void *opaque)
> > */
> > bdrv_get_backing_filename(s->target, backing_filename,
> > sizeof(backing_filename));
> > - if (backing_filename[0] && !s->target->backing) {
> > - ret = bdrv_get_info(s->target, &bdi);
> > - if (ret < 0) {
> > - goto immediate_exit;
> > - }
> > - if (s->granularity < bdi.cluster_size) {
> > - s->buf_size = MAX(s->buf_size, bdi.cluster_size);
> > - s->cow_bitmap = bitmap_new(length);
> > - }
> > + if (!bdrv_get_info(s->target, &bdi) && bdi.cluster_size) {
>
> This should be bdi.has_cluster_size...
has_cluster_size is a member of ImageInfo not BlockDriverInfo, and is derived
from (bdi.cluster_size != 0).
>
> > + target_cluster_size = bdi.cluster_size;
>
> ...and maybe we want an explicit minimum of BDRV_SECTOR_SIZE here; but I
> guess this is already assumed all over the block layer, so it may be
> fine without.
Okay, it doesn't hurt to add an assert here.
Fam
>
> > }
> > + if (backing_filename[0] && !s->target->backing
> > + && s->granularity < target_cluster_size) {
> > + s->buf_size = MAX(s->buf_size, target_cluster_size);
> > + s->cow_bitmap = bitmap_new(length);
> > + }
> > + s->target_cluster_sectors = target_cluster_size >> BDRV_SECTOR_BITS;
>
> (this shouldn't be 0, so target_cluster_size needs to be at least
> BDRV_SECTOR_SIZE)
>
> Max
>
