03.05.2018 17:39, Eric Blake wrote:
On 05/03/2018 04:20 AM, Vladimir Sementsov-Ogievskiy wrote:+ * `NBD_INFO_TRIM_SIZE` (4) ++ + - 16 bits, `NBD_INFO_TRIM_SIZE` + - 32 bits, minimum trim size + - 32 bits, maximum trim size + + * `NBD_INFO_ZERO_SIZE` (5) ++ + - 16 bits, `NBD_INFO_ZERO_SIZE` + - 32 bits, minimum zero size + - 32 bits, maximum zero size + * `NBD_REP_META_CONTEXT` (4)So, now we have two very similar options, and in realization - two almost identical code paths. Are we going to add similar limits for BLOCK_STATUS? and for CACHE? I have an idea: what about some kind of universal option for it? Something likeoption INFO_CMD_SIZE: specific limits for command: uint16_t command uint32_t min_block uint32_t max_blockand server can send several such options, client can request them in some generic way. Most of semantics for these additional limits are common, so it will simplify both documentation and realization.We already document "A particular information type SHOULD only appear once for a given export unless documented otherwise." - but this would indeed be a case where we could document otherwise.
Alternatively, we can save old behavior, by occupying a region of options, for example 100..200, and say that we have options INFO_*_SIZE = n+100, where * is command number n.. But it don't look like a good solution.
Do you envision a server that needs to have independent limits for CACHE and/or BLOCK_STATUS beyond the usual limits for READ, where advertising alternative limits is worthwhile?
CACHE: our planned usage (managed copy-on-read process) surely will benefit if client will not be restricted on caching request size.
for BLOCK_STATUS, when we use it in some copying process, it's good to query large region, to be able to use fast path in case of zero area (I don't remember, did we finally allowed last chunk of BLOCK_STATUS to exceed requested region?)
But the idea of having a single reusable info that documents per-command limits may be worth the effort, so I'll try a v5 of both this patch and of the qemu proof-of-concept implementation along those lines, so that we can compare the two approaches. Note, however, that if you only have a single command, a client can ONLY request NBD_INFO_CMD_SIZE once; it is then relying on the server to reply with as many per-command limits as the server wants, and can't specifically request alternative limits on a given command.Hmm, yes. We can either add a way to request an option with a parameter or live with it. But it don't look like a real problem, until we don't have thousands of commands)
-- Best regards, Vladimir