Re: [PATCH 2/3] uapi nbd: add cookie alias to handle

[Nir Soffer]

On Fri, Mar 10, 2023 at 10:16 PM Eric Blake <eblake@xxxxxxxxxx> wrote:
>
> The uapi <linux/nbd.h> header declares a 'char handle[8]' per request;
> which is overloaded in English (are you referring to "handle" the
> verb, such as handling a signal or writing a callback handler, or
> "handle" the noun, the value used in a lookup table to correlate a
> response back to the request).  Many client-side NBD implementations
> (both servers and clients) have instead used 'u64 cookie' or similar,
> as it is easier to directly assign an integer than to futz around with
> memcpy.  In fact, upstream documentation is now encouraging this shift
> in terminology: https://lists.debian.org/nbd/2023/03/msg00031.html
>
> Accomplish this by use of an anonymous union to provide the alias for
> anyone getting the definition from the uapi; this does not break
> existing clients, while exposing the nicer name for those who prefer
> it.  Note that block/nbd.c still uses the term handle (in fact, it
> actually combines a 32-bit cookie and a 32-bit tag into the 64-bit
> handle), but that internal usage is not changed the public uapi, since
> no compliant NBD server has any reason to inspect or alter the 64
> bits sent over the socket.
>
> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
> ---
>  include/uapi/linux/nbd.h | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/linux/nbd.h b/include/uapi/linux/nbd.h
> index 8797387caaf7..f58f2043f62e 100644
> --- a/include/uapi/linux/nbd.h
> +++ b/include/uapi/linux/nbd.h
> @@ -81,7 +81,10 @@ enum {
>  struct nbd_request {
>         __be32 magic;   /* NBD_REQUEST_MAGIC    */
>         __be32 type;    /* See NBD_CMD_*        */
> -       char handle[8];
> +       union {
> +               char handle[8];
> +               __be64 cookie;
> +       };
>         __be64 from;
>         __be32 len;
>  } __attribute__((packed));
> @@ -93,6 +96,9 @@ struct nbd_request {
>  struct nbd_reply {
>         __be32 magic;           /* NBD_REPLY_MAGIC      */
>         __be32 error;           /* 0 = ok, else error   */
> -       char handle[8];         /* handle you got from request  */
> +       union {
> +               char handle[8]; /* handle you got from request  */
> +               __be64 cookie;

Should we document like this?

    union {
        __be64 cookie; /* cookie you got from request */
        char handle[8]; /* older name */

I think we want future code to use the new term.

> +       };
>  };
>  #endif /* _UAPILINUX_NBD_H */
> --
> 2.39.2
>

Nir