[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [PATCH v2] proto: add xNBD command NBD_CMD_CACHE to the spec



On 05/04/2018 05:16 AM, Vladimir Sementsov-Ogievskiy wrote:

+    performance gain.  A future version of this standard may add
+    command flags to request particular caching behaviors, where a
+    server would reply with an error if that behavior cannot be
+    accomplished.
+
+    If an error occurs, the server MUST set the appropriate error code
+    in the error field. However failure on this operation does not
+    imply that further read and write requests on this area will fail.
+    When no command flags are in use, the server MAY send a reply
+    prior to the requested area being fully cached.
+
+    A client MAY attempt to send a cache request even when
+    `NBD_FLAG_SEND_CACHE` was not set in the transmission flags field,
+    however, in that case, it MUST NOT use any command flags.  A
+    server MAY advertise `NBD_FLAG_SEND_CACHE` even if the command has
+    no effect or even if the command always fails with `EINVAL`;
+    however, if it advertised the command, the server MUST reject any
+    command flags it does not recognize.
+


Sounds good. And you reserved flags for future usage. Just note: flags are globally reserved with SHOULD "The server SHOULD return `EINVAL` if it receives an unknown command flag", and you reinforced it with MUST.

Yes, that was intentional on my part - for many existing commands and/or implementations, we have to use the weaker language that a server SHOULD reject unknown flags (because not all historical implementations actually did so - after all, the reference implementation was only fixed in Mar 2016, per commit ab22e08); but for this command, we want it to be mandatory (if a server is new enough to implement the advertisement of this command, it should also be new enough to enforce correct flag handling on this particular command).

And it becomes important if we later extend the command by mapping particular flag values to various posix_fadvise() semantics. In the extreme case, if we add a command flag that maps to POSIX_FADV_DONTNEED semantics for dropping a region from the cache, then a server that does not understand the flag MUST fail the command, rather than doing the exact opposite of what the client requested by caching that region.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


Reply to: