Re: [Qemu-block] [Qemu-devel] [PATCH] nbd/server: Silence gcc false positive

[Eric Blake]

On 06/22/2018 07:58 AM, Eric Blake wrote:
> The code has a while() loop that always initialized 'end', and
> the loop always executes at least once (as evidenced by the assert()
> just prior to the loop).  But some versions of gcc still complain
> that 'end' is used uninitialized, so silence them.
>
> Signed-off-by: Eric Blake <address@hidden>
> ---
>
> Peter, do you want to apply this directly as a build fix, or shall
> I submit a pull request?

Phooey. I'm obviously under a lot of stress. In my rush to fix the build 
problem, I introduced a logic error that breaks NBD dirty bitmaps. :(

>   nbd/server.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/nbd/server.c b/nbd/server.c
> index 274604609f4..50ac8bfafc6 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -1937,7 +1937,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;
> @@ -1977,6 +1977,7 @@ static unsigned int bitmap_to_extents(BdrvDirtyBitmap 
> *bitmap, uint64_t offset,
>
>       bdrv_dirty_bitmap_unlock(bitmap);
>
> +    assert(offset > end);
>       *length = end - offset;

The assertion is backwards; that should be 'offset < end'. :(

Sending another fix. And putting on the brown paper bag of shame.

And bummer that neither 'make check' nor iotests exposes the breakage. 
My RFC hack for adding x-block-status to the client would be useful for 
that purpose.

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