Re: [Qemu-devel] [PULL v2 0/9] bitmap export over NBD

[Eric Blake]

On 06/22/2018 06:43 AM, Peter Maydell wrote:
> On 22 June 2018 at 10:57, Peter Maydell <address@hidden> wrote:
> > On 21 June 2018 at 15:57, Eric Blake <address@hidden> wrote:
> > > ----------------------------------------------------------------
> > > nbd patches for 2018-06-20
> > >
> > > Add experimental x-nbd-server-add-bitmap to expose a disabled
> > > bitmap over NBD, in preparation for a pull model incremental
> > > backup scheme. Also fix a corner case protocol issue with
> > > NBD_CMD_BLOCK_STATUS, and add new NBD_CMD_CACHE.
> > >
> > > - Eric Blake: tests: Simplify .gitignore
> > > - Eric Blake: nbd/server: Reject 0-length block status request
> > > - Vladimir Sementsov-Ogievskiy: 0/6 NBD export bitmaps
> > > - Vladimir Sementsov-Ogievskiy: nbd/server: introduce NBD_CMD_CACHE
> > Applied, thanks.
>
> ...patchew seems to be unhappy, though, eg
> https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg06559.html
> has an unrelated patchset failing with:
>
> /tmp/qemu-test/src/nbd/server.c: In function 'nbd_trip':
> /tmp/qemu-test/src/nbd/server.c:1980:19: error: 'end' may be used uninitialized
> in this function [-Werror=maybe-uninitialized]
>       *length = end - offset;
>                 ~~~~^~~~~~~~

Uurgh. It's a false positive (the compiler is complaining that the 
variable is uninitialized, which can only happen if the while loop is 
not executed; but the preconditions guarantee the loop executes at least 
once).  The assert() that I added was enough to silence gcc 7.3.1 on my 
Fedora 27 system, but address@hidden is using 
gcc-8.1.1-1.fc28.x86_64. This should silence things (another way to 
silence would be rewriting while{} into do{}while).  I'll submit this as 
a formal patch if I can reproduce the problem/fix on docker.

diff --git i/nbd/server.c w/nbd/server.c
index 94137fbfe8f..2379f82d5d4 100644
--- i/nbd/server.c
+++ w/nbd/server.c
@@ -1968,7 +1968,7 @@ static unsigned int 
bitmap_to_extents(BdrvDirtyBitmap *bitmap, uint64_t offset,
                                       unsigned int nb_extents,
                                       bool dont_fragment)
 {
-    uint64_t begin = offset, end;
+    uint64_t begin = offset, end = offset;
     uint64_t overall_end = offset + *length;
     unsigned int i = 0;
     BdrvDirtyBitmapIter *it;
@@ -2008,6 +2008,7 @@ static unsigned int 
bitmap_to_extents(BdrvDirtyBitmap *bitmap, uint64_t offset,

     bdrv_dirty_bitmap_unlock(bitmap);

+    assert(offset > end);
     *length = end - offset;
     return i;
 }


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