14.09.2018 20:35, Eric Blake wrote:
On 9/14/18 12:30 PM, Vladimir Sementsov-Ogievskiy wrote:
while (begin < overall_end && i < nb_extents) {
+ bool next_dirty = !dirty;
+
if (dirty) {
end = bdrv_dirty_bitmap_next_zero(bitmap, begin);
} else {
@@ -1962,6 +1964,7 @@ static unsigned int
bitmap_to_extents(BdrvDirtyBitmap *bitmap, uint64_t offset,
end = MIN(bdrv_dirty_bitmap_size(bitmap),
begin + UINT32_MAX + 1 -
bdrv_dirty_bitmap_granularity(bitmap));
+ next_dirty = dirty;
}
Rather than introducing next_dirty, couldn't you just make this:
if (end == -1 || end - begin > UINT32_MAX) {
/* Cap ... */
end = MIN(...);
} else {
dirty = !dirty;
}
no, dirty variable is used after it, we can't change it here.
Ah, right. But we could also hoist the extents[i].flags =
cpu_to_be32(dirty ? NBD_STATEE_DIRTY : 0) line to occur prior to the
'if' doing the end capping calculation. However, splitting the two
assignments into extents[i].* to no longer be consecutive statements,
just to avoid the use of a temporary variable, starts to get less
aesthetically pleasing.
Thus, I'm fine with your version (with commit message improved),
unless you want to send a v2.
Ok, I don't want:) Be free to update commit message and pull it.
Reviewed-by: Eric Blake <ebl...@redhat.com>
--
Best regards,
Vladimir
