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 <ebl...@redhat.com> > >> Reviewed-by: John Snow <js...@redhat.com> > >> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@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
signature.asc
Description: PGP signature