Re: [Qemu-block] [PATCH 6/9] nbd: BLOCK_STATUS for standard get_block_status function: client part

[Vladimir Sementsov-Ogievskiy]

16.02.2018 23:40, Eric Blake wrote:

On 02/15/2018 07:51 AM, Vladimir Sementsov-Ogievskiy wrote:

Minimal realization: only one extent in server answer is supported.
Flag NBD_CMD_FLAG_REQ_ONE is used to force this behavior.

Tests 140, 147 and 205 are fixed due to now server failed on searching
export in context of NBD_OPT_SET_META_CONTEXT option negotiation.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
---

[...]

Instead of doing a memcpy() and then in-place bit-swizzling, you could do the swapping as part of assignment, for one less function call (and make the code a bit easier to extend, if we later drop our REQ_ONE limitation on only having one extent, because you'll advance payload as needed):

extent->length = payload_advance32(&payload);
extent->flags = payload_advance32(&payload);

We should probably validate that the length field is a multiple of min_block (if a server tells us that all operations must be 512-byte aligned, then reports an extent that is smaller than 512 bytes, we have no way to ask for the status of the second half of the sector). Probably also something that needs to be explicitly stated in the NBD spec. [1]

related question: what server should reply on zero-length request? I'll add

+     if (!bytes) {                                                               
+         *pnum = 0;                                                              
+         return 0;                                                               
+     }  

to nbd_client_co_block_status, to prevent such situation, but looks like spec lacks the information.

-- 
Best regards,
Vladimir