Le 06/02/2020 à 18:38, Eric Blake a écrit :
> Detected by a hang in the libnbd testsuite. If a client requests
> multiple meta contexts (both base:allocation and qemu:dirty-bitmap:x)
> at the same time, our attempt to silence a false-positive warning
> about a potential uninitialized variable introduced botched logic: we
> were short-circuiting the second context, and never sending the
> NBD_REPLY_FLAG_DONE. Combining two 'if' into one 'if/else' in
> bdf200a55 was wrong (I'm a bit embarrassed that such a change was my
> initial suggestion after the v1 patch, then I did not review the v2
> patch that actually got committed). Revert that, and instead silence
> the false positive warning by replacing 'return ret' with 'return 0'
> (the value it always has at that point in the code, even though it
> eluded the deduction abilities of the robot that reported the false
> positive).
>
> Fixes: bdf200a5535
> Signed-off-by: Eric Blake <[email protected]>
> ---
>
> It's never fun when a regression is caused by a patch taken through
> qemu-trivial, proving that the patch was not trivial after all.
trivial doesn't mean not reviewed...
The patch v1 was trivial, the v2 wasn't.
>
> nbd/server.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/nbd/server.c b/nbd/server.c
> index 87fcd2e7bfac..11a31094ff83 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -2384,15 +2384,23 @@ static coroutine_fn int nbd_handle_request(NBDClient
> *client,
> !client->export_meta.bitmap,
> NBD_META_ID_BASE_ALLOCATION,
> errp);
> - } else { /* client->export_meta.bitmap */
> + if (ret < 0) {
> + return ret;
> + }
> + }
> +
> + if (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;
> + }
> }
>
> - return ret;
> + return 0;
> } else {
> return nbd_send_generic_reply(client, request->handle, -EINVAL,
> "CMD_BLOCK_STATUS not negotiated",
>
Reviewed-by: Laurent Vivier <[email protected]>
