[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 16/18] qcow2: Switch store_bitmap_data() to b
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v6 16/18] qcow2: Switch store_bitmap_data() to byte-based iteration |
Date: |
Fri, 8 Sep 2017 16:16:29 +0200 |
User-agent: |
Mutt/1.8.3 (2017-05-23) |
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 <address@hidden>
> >> Reviewed-by: John Snow <address@hidden>
> >> Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> >>
>
> >>
> >> - 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