On Tue, 04/19 22:33, Max Reitz wrote:
> On 19.04.2016 12:09, Fam Zheng wrote:
> > The last sub-chunk is rounded up to the copy granularity in the target
> > image, resulting in a larger size than the source.
> >
> > Add a function to clip the copied sectors to the end.
> >
> > This undoes the "wrong" changes to tests/qemu-iotests/109.out in
> > e5b43573e28. The remaining two offset changes are okay.
> >
> > Reported-by: Kevin Wolf <kw...@redhat.com>
> > Signed-off-by: Fam Zheng <f...@redhat.com>
> > ---
> > block/mirror.c | 10 ++++++++++
> > tests/qemu-iotests/109.out | 44
> > ++++++++++++++++++++++----------------------
> > 2 files changed, 32 insertions(+), 22 deletions(-)
> >
> > diff --git a/block/mirror.c b/block/mirror.c
> > index c2cfc1a..b6387f1 100644
> > --- a/block/mirror.c
> > +++ b/block/mirror.c
> > @@ -205,6 +205,14 @@ static inline void mirror_wait_for_io(MirrorBlockJob
> > *s)
> > s->waiting_for_io = false;
> > }
> >
> > +static inline void mirror_clip_sectors(MirrorBlockJob *s,
> > + int64_t sector_num,
> > + int *nb_sectors)
> > +{
> > + *nb_sectors = MIN(*nb_sectors,
> > + s->bdev_length / BDRV_SECTOR_SIZE - sector_num);
> > +}
> > +
> > /* Submit async read while handling COW.
> > * Returns: nb_sectors if no alignment is necessary, or
> > * (new_end - sector_num) if tail is rounded up or down due to
> > @@ -240,6 +248,7 @@ static int mirror_do_read(MirrorBlockJob *s, int64_t
> > sector_num,
> > mirror_wait_for_io(s);
> > }
> >
> > + mirror_clip_sectors(s, sector_num, &nb_sectors);
> > /* Allocate a MirrorOp that is used as an AIO callback. */
> > op = g_new(MirrorOp, 1);
> > op->s = s;
>
> I think you want to adjust the ret value, too. It doesn't really matter
> in practice (the caller just overshoots the end of the image instead of
> getting precisely to its end), but I wouldn't rely on this.
>
> > @@ -276,6 +285,7 @@ static void mirror_do_zero_or_discard(MirrorBlockJob *s,
> > {
> > MirrorOp *op;
> >
> > + mirror_clip_sectors(s, sector_num, &nb_sectors);
> > /* Allocate a MirrorOp that is used as an AIO callback. The qiov is
> > zeroed
> > * so the freeing in mirror_iteration_done is nop. */
> > op = g_new0(MirrorOp, 1);
>
> I think it would be best to just pull out the mirror_clip_sectors() from
> these functions and put it above the "switch (mirror_method)" in
> mirror_iteration().
>
> We'd just have to make sure that mirror_cow_align() will not increase
> nb_sectors such that it points beyond the image end. It can do that,
> because and image's size does not need to be aligned to its cluster
> size. But just putting a mirror_clip_sectors() below the
> bdrv_round_to_clusters() call in mirror_cow_align() should fix that.
>
> Then you wouldn't need to worry about fixing the ret value in
> mirror_do_read().
Sounds good, will do this.
Thanks,
Fam
