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

Re: [PATCH 1/2] nbd/proto: introduce structured request



06.02.2020 16:05, Eric Blake wrote:
On 2/5/20 10:43 AM, Vladimir Sementsov-Ogievskiy wrote:
Idea: reuse structured replies as requests too. For this:

Rename structured reply "structure" to structured message. And,
correspondingly structured reply chunk to structured message chunk.

Keep name "structured reply" for structured messages sent by server,
and name "structured request" a structured message sent by client.

Share almost all semantics around structured messages for client and
server except for chunk type (as reply types and request types are
orthogonal things). Still, share none-type chunk for both server and
client.


Without looking at the doc patch itself, my initial worry is as follows:

For structured replies, we needed the ability to allow a server to have split replies (sparse reads were the primary reason behind that decision - the client does not know a priori which portions of the image are sparse, and the server needs some way to represent both data and holes each as efficiently as possible over the wire for a single read request).  On the client side, this means that either the client has to track enough state to reassemble multiple server messages intermixed with one another, or the client can choose to serialize things so that the server only ever has at most one outstanding command.  On the server side, some servers may already be smart enough to handle interleaved client requests such that structured replies can indeed end up interleaved, while others may serialize how they react to incoming client messages and send all chunks of a given reply (in the few cases where sending more than one chunk even makes sense) before parsing the next client message.  Thus, while the protocol permits interleaving, it does not force either side to implement interleaving.

But if we allow structured requests to be split across multiple packets, we either have to mandate that clients CANNOT split requests across multiple packets, or we are FORCING the server to track additional state to deal with clients that fragment and interleave requests, even if most clients don't do that.

I understand this and I don't wont to implement such logic in a qemu nbd server now. So, I suggest to allow/disallow splitting per structured chunk type. If we define only WRITE_ZEROES64 chunk type and state that it must be the only chunk in the structured requests, nobody should implement splitted request handling in server.

While split replies make sense, I'm having a hard time seeing how split requests would ever be useful (a client does not need to try to do a write with holes in a single request; instead, write the data and write the holes via two separate write requests over smaller portions of the export).  So while we may want to reuse the magic number and on-the-wire format of a structured message, I'd strongly argue that structured requests must be documented in a way that makes it obvious that they will always occupy exactly one chunk (reserving multiple chunks and the need for the DONE flag only for replies).


Hmm, don't you think still, that structured writes may be useful at some point? Now we always translate structured read reply to real allocated in RAM zeroes. But at some point we may instead propagate this concept to the generic block layer, and make support for it in qcow2. Or even with support only in NBD we may benefit if we are coping data from one NBD to andother: we should not unpack zeroes.

Still, I don't want to implement it now, and I feel that block-status driven copying does the same and already works.

I agree, that now it's better to restrict multiple chunks in requests, and I'll resend with this in mind. Still, I seems good to document it so we'll be able to add multiple chunks support in future with no extra pain..


--
Best regards,
Vladimir


Reply to: