On Tue, Mar 14, 2023 at 02:50:23PM -0500, Eric Blake wrote: > On Sat, Mar 11, 2023 at 02:30:39PM +0200, Nir Soffer wrote: > > 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. > > Sure, swapping the order to favor the preferred name first makes sense. > > I'm still not sure on whether cookie should be u64 or __be64 (it's > opaque, so endianness over the wire doesn't matter; I guess it is 'u64', given ->handle is always copied to nbd_reply from nbd_request in nbd server side, so native endian is always applied for building and parsing ->handle in nbd client side. But it looks odd to mark it as u64. > and previous code > was using memcpy() onto char[8] which may behave differently depending > on machine endianness). Thanks, Ming
