[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-devel] [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 -
+            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,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]