|
From: | Vladimir Sementsov-Ogievskiy |
Subject: | Re: [Qemu-block] [PATCH] nbd/server: fix bitmap export |
Date: | Fri, 14 Sep 2018 20:47:08 +0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
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 <address@hidden>
-- Best regards, Vladimir
[Prev in Thread] | Current Thread | [Next in Thread] |