Hi guys,
Sorry for slacking here.
On Wed, Mar 18, 2020 at 07:22:39AM -0500, Eric Blake wrote:
On 3/18/20 3:04 AM, Wouter Verhelst wrote:
On Wed, Mar 18, 2020 at 09:19:45AM +0300, Vladimir Sementsov-Ogievskiy wrote:
OK, understand. Reasonable thing, agreed. I'll resend.
Hmm. But we can't read in one go anyway, because we need to distinguish
NBD_REQUEST_MAGIC
from NBD_EXTENDED_REQUEST_MAGIC..
Yes, that needs to happen at any rate, indeed. So the difference will be
two reads rather than three, instead of one read rather than two.
That's still an advantage.
Not much of one. You're micro-optimizing the read()s, but in reality, the
sender will likely send() the entire packet at once, at which point the data
is in the kernel and the reads will succeed back-to-back, or you can even
write the client to read into a buffer to minimize syscalls and then parse
as much as needed out of the buffer.
You've got a LOT more overhead in the TCP packet and network transmission
time than you do in deciding whether the server has to do 2 or 3 read()s per
client message.
While it might be nice to design a message that doesn't need the server to
do additional decision points mid-packet in determining how much packet
remains, that should not be your driving factor. Even with current servers,
that is already the case (the server has to decide mid-packet whether it is
handling NBD_CMD_WRITE and thus has more data to read).
Having payload length and affected length as separate entities also
makes the protocol context free, which should make things easier to
parse for things like wireshark etc. It also feels a bit cleaner to me,
in that a server implementation can separate "reading data" from
"handling data" more cleanly.
(e.g., nbd-server has to special-case NBD_CMD_WRITE in a generic "read
the next command" function, which I think is rather ugly)
Other than this (minor) issue, I think this proposal is certainly ready
to go, and I apologise for not having followed up on it ealier.
Would you be so kind as to propose a new patch, with changes as
suggested in this thread? I promise to not let it linger for months
again then ;-)