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

[Eric Blake]

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