Re: [Libguestfs] [PATCH v7 01/12] nbd/server: Support a request payload

On Thu, Sep 28, 2023 at 12:09:51PM +0300, Vladimir Sementsov-Ogievskiy wrote: > On 27.09.23 18:59, Eric Blake wrote: >> We could also try to be a bit more complicated by peeking at the next >> few bytes: if they look like a magic number of the next request, >> assume the client set the bit accidentally but didn't send a payload >> after all; for anything else, assume the client did pass a payload. >> But adding in machinery to peek at a prefix is more complex than >> either assuming a payload is always present (as done in this patch) or >> assuming the bit was in error (and dropping the connection >> unconditionally). Preferences? > > > Ohh, you are right, thanks for comprehensive explanation. I really missed some things you are saying about. Yes, now I agree that "payload always exist when flag is set" is the best effort. Finally, that was our aim of the protocol design: make it more context independent. Probably, we may fix that in specification as preferable or at least possible server behavior about non-compliant client. One other possibility I just thought of: have a heuristic where the flag set with h->request_length less than 512 bytes is likely to indicate an intentional payload (even if for a command where we weren't expecting payload, so still a client error); while the flag set wtih h->request_length >= 512 bytes is likely to be a mistaken setting of the flag (but also still a client error). NBD_CMD_WRITE is probably the only command that will ever need to send a payload larger than one sector, but that command already has handling to accept payloads of all sizes because we know what to do with them and where the client is not in error.