Re: [Qemu-devel] [PATCH] nbd/server: fix bitmap export

[Eric Blake <eblake@redhat.com>]

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 <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org