Re: [Qemu-block] [PATCH v8 19/21] vpc: Switch to .bdrv_co_block_status()

[Eric Blake]

On 02/14/2018 07:08 AM, Kevin Wolf wrote:
> Am 13.02.2018 um 21:26 hat Eric Blake geschrieben:
> > We are gradually moving away from sector-based interfaces, towards
> > byte-based.  Update the vpc driver accordingly.
> >
> > Signed-off-by: Eric Blake <address@hidden>
> > Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> > Reviewed-by: Fam Zheng <address@hidden>
> >
> > ---

> > +    allocated = (image_offset != -1);
> >       *pnum = 0;
> >       ret = 0;
> >
> >       do {
> >           /* All sectors in a block are contiguous (without using the bitmap) */
> > -        n = ROUND_UP(sector_num + 1, s->block_size / BDRV_SECTOR_SIZE)
> > -          - sector_num;
> > -        n = MIN(n, nb_sectors);
> > +        n = ROUND_UP(offset + 1, s->block_size) - offset;
> > +        n = MIN(n, bytes);
> >
> >           *pnum += n;
> > -        sector_num += n;
> > -        nb_sectors -= n;
> > +        offset += n;
> > +        bytes -= n;
> >           /* *pnum can't be greater than one block for allocated
> >            * sectors since there is always a bitmap in between. */
> >           if (allocated) {
> >               *file = bs->file->bs;
> > -            ret = BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start;
> > +            *map = image_offset;
>
> This does work, but only because the loop isn't actually looping for
> allocated == true. In the old code, it was obvious that start was
> assigned only once and never changed during the loop, but image_offset
> changes in each loop iteration.
>
> It would probably be cleaner and more obviously correct to move the
> assignment of *map to before the loop.

Yes, that would be a bit nicer.

> Kevin

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org