Re: [Qemu-devel] [PATCH 2/3] NBD proto: document additional error conditions

[Eric Blake]

On 03/28/2016 04:43 AM, Denis V. Lunev wrote:
> From: Pavel Borzenkov <address@hidden>
> 
> It is unclear what the behaviour of a server should be if it receives
> an unknown command. Similar uncertainty exists for command flags.
> 
> Make it explicit that the server should return EINVAL in all such cases.

I'd go one step further and document that for backwards compatibility,
clients should not rely on this particular error.

> 
> Signed-off-by: Pavel Borzenkov <address@hidden>
> Reviewed-by: Roman Kagan <address@hidden>
> Signed-off-by: Denis V. Lunev <address@hidden>
> CC: Wouter Verhelst <address@hidden>
> CC: Eric Blake <address@hidden>
> CC: Alex Bligh <address@hidden>
> ---
>  doc/proto.md | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/doc/proto.md b/doc/proto.md
> index d54ed19..036d6d9 100644
> --- a/doc/proto.md
> +++ b/doc/proto.md
> @@ -512,6 +512,13 @@ return `EINVAL` if it receives a read or trim request 
> including one or
>  more sectors beyond the size of the device.  It also SHOULD map the
>  `EDQUOT` and `EFBIG` errors to `ENOSPC`.  Finally, it SHOULD return
>  `EPERM` if it receives a write or trim request on a read-only export.
> +
> +The server SHOULD return `EINVAL` if it receives an unknown command.
> +
> +The server SHOULD return `EINVAL` if it receives an unknown command flag. It
> +also SHOULD return `EINVAL` if it receives a request with a flag not 
> explicitly
> +documented as applicable to the given request.

In other words, while this is good for the server side, we have proven
that existing server implementations did not follow this, and therefore
it would probably be good to add a sentence along the lines of:

However, clients should not rely on this particular error, as some
historical servers silently ignored invalid commands or flags.

But as what you have proposed is a strict improvement,
Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature