Re: [Qemu-devel] [PATCH v6 16/18] qcow2: Switch store_bitmap_data() to byte-based iteration

[Kevin Wolf <kwolf@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1424 bytes --]

Am 08.09.2017 um 16:09 hat Eric Blake geschrieben:
> On 09/08/2017 08:27 AM, Kevin Wolf wrote:
> > Am 30.08.2017 um 23:05 hat Eric Blake geschrieben:
> >> Now that we have adjusted the majority of the calls this function
> >> makes to be byte-based, it is easier to read the code if it makes
> >> passes over the image using bytes rather than sectors.
> >>
> >> Signed-off-by: Eric Blake <eblake@redhat.com>
> >> Reviewed-by: John Snow <jsnow@redhat.com>
> >> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> >>
> 
> >>
> >> -    while ((sector = bdrv_dirty_iter_next(dbi) >> BDRV_SECTOR_BITS) != -1) {
> >> -        uint64_t cluster = sector / sbc;
> >> +    while ((offset = bdrv_dirty_iter_next(dbi)) != -1) {
> > 
> > Don't you have to multiply both sides of the equation? This would be
> > offset != -512, which points out that the previous patch to convert
> > bdrv_dirty_iter_next() to byte-based gave it a really awkward interface.
> 
> I think what I really need to do is change '!= -1' to '< 0', as that's
> much easier to reason about when scaling is present.

Hm, I think I would prefer a special case for -1 in bdrv_dirty_iter_next()
so that it returns -1 after the last entry. Even if you check for < 0,
-512 is still an odd return value to signal the end.

Though I think after the final patch, we're back to -1 anyway, so it's
not that important.

Kevin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]