Re: [Qemu-block] [PATCH for-4.0] nbd: Permit simple error to NBD_CMD_BLOCK_STATUS

[Eric Blake]

On 3/25/19 11:21 AM, Eric Blake wrote:

> When you fix the first bug (the client setting iter.err on a simple
> error reply from the server because the server's reply wasn't
> structured, to now the client setting just iter.request_ret because it
> successfully parsed an error out of the server's reply), this code is
> now reachable with iter.err == NULL. Since the server replied with an
> error, extent->length is 0. So this code (added in 7f86068d) says that
> since iter.err is not set, we give up on the server and hang up (but
> only for simple errors, not for structured errors). It is a regression,
> because prior to 7f86068d, we did not have iter.request_ret, and instead
> went by the presence or absence of iter.fatal, and iter.fatal was not
> set when dealing with a simple error when a structured error was supplied.
> 
> So, the full setup of things to test:
> 
> server that always fails with structured error:
>  - client before 7f86068d (qemu 3.1): connection stays alive, regardless
> of this patch
>  - client after 7f86068d: connection stays alive, regardless of this patch
> 
> server that always fails with simple error:
>  - client before 7f86068d: (second half of patch does not apply, so only
> the first half is needed):
>    - without first half, connection dies because "Protocol error: simple
> reply when structured reply chunk was expected"

What's really annoying is that iter.err does not show up anywhere except
maybe in trace messages; on stderr, the symptoms are merely:

Failed to get allocation status: Invalid argument

because nbd_co_do_receive_one_chunk returns -EINVAL for iter.ret.

>    - with first half, connection stays alive as it does for structured error
>  - client after 7f86068d, with various stages of this patch:
>    - without either half (or with second half only), connection dies
> because "Protocol error: simple reply when structured reply chunk was
> expected"

again, shows up only as

Failed to get allocation status: Invalid argument

>    - with first half, connection dies because "server did not reply with
> any status extents"

Here, shows up only as

Failed to get allocation status: Input/output error

because without the second hunk, iter.ret is set to -EIO.

>    - with both halves, connection stays alive
> 

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

signature.asc
Description: OpenPGP digital signature