qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Laurent Vivier <laurent@vivier.eu>, qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
	qemu-block@nongnu.org, qemu-trivial@nongnu.org,
	Pan Nengyuan <pannengyuan@huawei.com>,
	Michael Tokarev <mjt@tls.msk.ru>,
	qemu-arm@nongnu.org, Euler Robot <euler.robot@huawei.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Richard Henderson <richard.henderson@linaro.org>
Subject: Re: [PULL 3/5] nbd: fix uninitialized variable warning
Date: Thu, 6 Feb 2020 10:55:06 -0600	[thread overview]
Message-ID: <1ce40cae-eb8b-a18c-f651-2e6432c1b213@redhat.com> (raw)
In-Reply-To: <20200108160233.991134-4-laurent@vivier.eu>

On 1/8/20 10:02 AM, Laurent Vivier wrote:
> From: Pan Nengyuan <pannengyuan@huawei.com>
> 
> Fixes:
> /mnt/sdb/qemu/nbd/server.c: In function 'nbd_handle_request':
> /mnt/sdb/qemu/nbd/server.c:2313:9: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>       int ret;
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> Message-Id: <20200108025132.46956-1-pannengyuan@huawei.com>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>   nbd/server.c | 10 +---------
>   1 file changed, 1 insertion(+), 9 deletions(-)
> 

This patch is broken; I first noticed it when testing libnbd, where the 
symptoms are a hung interop/dirty-bitmap.sh test.

> diff --git a/nbd/server.c b/nbd/server.c
> index 24ebc1a80571..87fcd2e7bfac 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -2384,20 +2384,12 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
>                                                  !client->export_meta.bitmap,
>                                                  NBD_META_ID_BASE_ALLOCATION,
>                                                  errp);
> -                if (ret < 0) {
> -                    return ret;
> -                }
> -            }
> -
> -            if (client->export_meta.bitmap) {
> +            } else {              /* client->export_meta.bitmap */
>                   ret = nbd_co_send_bitmap(client, request->handle,
>                                            client->exp->export_bitmap,
>                                            request->from, request->len,
>                                            dont_fragment,
>                                            true, NBD_META_ID_DIRTY_BITMAP, errp);
> -                if (ret < 0) {
> -                    return ret;
> -                }
>               }

The NBD spec says that clients can request more than one meta context at 
a time. Qemu does not (and hence we didn't notice the breakage until 
now), but libnbd does, and this change prevents qemu from obeying the 
spec (if the client requested both contexts, then we must return both, 
and not short-circuit after the first).

I'll post a patch to fix this unintended regression soon.

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



  reply	other threads:[~2020-02-06 16:56 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-08 16:02 [PULL 0/5] Trivial branch patches Laurent Vivier
2020-01-08 16:02 ` [PULL 1/5] MAINTAINERS: Update Yuval Shaia's email address Laurent Vivier
2020-01-08 16:02 ` [PULL 2/5] util/module: fix a memory leak Laurent Vivier
2020-01-08 16:02 ` [PULL 3/5] nbd: fix uninitialized variable warning Laurent Vivier
2020-02-06 16:55   ` Eric Blake [this message]
2020-01-08 16:02 ` [PULL 4/5] arm/translate-a64: " Laurent Vivier
2020-01-08 16:02 ` [PULL 5/5] vl: fix memory leak in configure_accelerators Laurent Vivier
2020-01-13 11:41 ` [PULL 0/5] Trivial branch patches Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1ce40cae-eb8b-a18c-f651-2e6432c1b213@redhat.com \
    --to=eblake@redhat.com \
    --cc=euler.robot@huawei.com \
    --cc=laurent@vivier.eu \
    --cc=mjt@tls.msk.ru \
    --cc=pannengyuan@huawei.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).