[Libguestfs] [nbdkit PATCH 3/7] RFC: protocol: Only send EOVERFLOW when valid

[Richard W.M. Jones]

On Mon, Apr 22, 2019 at 07:50:22PM -0500, Eric Blake wrote:
> Previously, we were squashing EOVERFLOW into EINVAL; continue to do so
> at points in the protocol where the client may not be expecting
> EOVERFLOW.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  server/protocol.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/server/protocol.c b/server/protocol.c
> index a52bb56..0a9f73c 100644
> --- a/server/protocol.c
> +++ b/server/protocol.c
> @@ -326,7 +326,7 @@ skip_over_write_buffer (int sock, size_t count)
> 
>  /* Convert a system errno to an NBD_E* error code. */
>  static int
> -nbd_errno (int error)
> +nbd_errno (int error, bool flag_df)
>  {
>    switch (error) {
>    case 0:
> @@ -349,7 +349,9 @@ nbd_errno (int error)
>      return NBD_ESHUTDOWN;
>  #endif
>    case EOVERFLOW:
> -    return NBD_EOVERFLOW;
> +    if (flag_df)
> +      return NBD_EOVERFLOW;
> +    /* fallthrough */
>    case EINVAL:
>    default:
>      return NBD_EINVAL;
> @@ -368,7 +370,7 @@ send_simple_reply (struct connection *conn,
> 
>    reply.magic = htobe32 (NBD_SIMPLE_REPLY_MAGIC);
>    reply.handle = handle;
> -  reply.error = htobe32 (nbd_errno (error));
> +  reply.error = htobe32 (nbd_errno (error, false));
> 
>    r = conn->send (conn, &reply, sizeof reply);
>    if (r == -1) {
> @@ -573,7 +575,8 @@ send_structured_reply_block_status (struct connection *conn,
> 
>  static int
>  send_structured_reply_error (struct connection *conn,
> -                             uint64_t handle, uint16_t cmd, uint32_t error)
> +                             uint64_t handle, uint16_t cmd, uint16_t flags,
> +                             uint32_t error)
>  {
>    ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock);
>    struct structured_reply reply;
> @@ -593,7 +596,7 @@ send_structured_reply_error (struct connection *conn,
>    }
> 
>    /* Send the error. */
> -  error_data.error = htobe32 (nbd_errno (error));
> +  error_data.error = htobe32 (nbd_errno (error, flags & NBD_CMD_FLAG_DF));
>    error_data.len = htobe16 (0);
>    r = conn->send (conn, &error_data, sizeof error_data);
>    if (r == -1) {
> @@ -737,7 +740,8 @@ protocol_recv_request_send_reply (struct connection *conn)
>                                                     extents);
>      }
>      else
> -      return send_structured_reply_error (conn, request.handle, cmd, error);
> +      return send_structured_reply_error (conn, request.handle, cmd, flags,
> +                                          error);
>    }
>    else
>      return send_simple_reply (conn, request.handle, cmd, buf, count, error);
> -- 
> 2.20.1

The protocol spec is unclear on whether EOVERFLOW can be returned in
cases other than the DF flag being set.  Whether we include this patch
or not seems to hinge on the interpretation of the protocol spec which
I'm not really in a position to make.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW