|
From: | Eric Blake |
Subject: | Re: [Qemu-devel] [PATCH] nbd/server: fix bitmap export |
Date: | Fri, 14 Sep 2018 12:35:08 -0500 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0 |
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.
Reviewed-by: Eric Blake <address@hidden> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[Prev in Thread] | Current Thread | [Next in Thread] |