Re: [Qemu-devel] [PATCH 1/1] nbd: fix NBD_CMD_CACHE negitiation according to the NBD specification

[Denis V . Lunev]

s/negitiation/negotiation/

On 10/04/2018 01:03 PM, Denis V. Lunev wrote:
> Commit bc37b06a5 was made very bad thing, it has been added
> NBD_FLAG_SEND_CACHE flag for negotiation. The problem is that the value
> of the flag was taken wrong and directly violates NBD specification.
> This value (bit 8) is used at least in the Linux kernel as a part of
> stable userspace-kernel API since 4.10 as NBD_FLAG_CAN_MULTI_CONN
> as defined in the specification:
>
> "bit 8, NBD_FLAG_CAN_MULTI_CONN: Indicates that the server operates
> entirely without cache, or that the cache it uses is shared among all
> connections to the given device. In particular, if this flag is
> present, then the effects of NBD_CMD_FLUSH and NBD_CMD_FLAG_FUA
> MUST be visible across all connections when the server sends its reply
> to that command to the client. In the absense of this flag, clients
> SHOULD NOT multiplex their commands over more than one connection to
> the export.
> ...
> bit 10, NBD_FLAG_SEND_CACHE: documents that the server understands
> NBD_CMD_CACHE; however, note that server implementations exist
> which support the command without advertising this bit, and
> conversely that this bit does not guarantee that the command will
> succeed or have an impact."
>
> Personally I do not see any option that we will be allowed to fix the
> specification in favor of QEMU and apply the fix to the kernel. This
> is a big-big problem. Let us fix this in QEMU as soon as possible.
>
> Thus the commit fixes negotiation flag in QEMU accoring to the
> specification. Fortunately enough the bit has been added by Virtuozzo
> and there are no released products in the field with this bit used
> so far.
>
> Signed-off-by: Denis V. Lunev <address@hidden>
> CC: Vladimir Sementsov-Ogievskiy <address@hidden>
> CC: Valery Vdovin <address@hidden>
> CC: Eric Blake <address@hidden>
> CC: Paolo Bonzini <address@hidden>
> ---
>  include/block/nbd.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/block/nbd.h b/include/block/nbd.h
> index 4638c839f5..4377fa502c 100644
> --- a/include/block/nbd.h
> +++ b/include/block/nbd.h
> @@ -135,7 +135,7 @@ typedef struct NBDExtent {
>  #define NBD_FLAG_SEND_TRIM         (1 << 5) /* Send TRIM (discard) */
>  #define NBD_FLAG_SEND_WRITE_ZEROES (1 << 6) /* Send WRITE_ZEROES */
>  #define NBD_FLAG_SEND_DF           (1 << 7) /* Send DF (Do not Fragment) */
> -#define NBD_FLAG_SEND_CACHE        (1 << 8) /* Send CACHE (prefetch) */
> +#define NBD_FLAG_SEND_CACHE        (1 << 10) /* Send CACHE (prefetch) */
>  
>  /* New-style handshake (global) flags, sent from server to client, and
>     control what will happen during handshake phase. */