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

[Eric Blake]

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.

> 
> r-b coming soon, I just need to take another look with corrected picture in 
> mind.
> 
> -- 
> Best regards,
> Vladimir
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org