Re: [Qemu-devel] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3724 bytes --]

On 03/24/2016 06:30 AM, Pavel Borzenkov wrote:
>> Conversely, it would be possible to send less data over the wire, as
>> long as we require that all LBA status descriptors cover consecutive
>> offsets.  That is, having the server reply with offsets is pointless,
>> since they can be reconstructed on the client by starting with the
>> offset in the client's request, then adding length from each status
>> field.  Is less network traffic desirable?
> 
> I think it's better to explicitly send the start of each LBA extent.

Why? Is the redundancy for something that the client can reconstruct
worth the extra safety at the cost of more traffic?

> So I'll go with changing 'status' field to 32 bits to avoid
> packing/unpacking issues.

You may want to do that even if you eliminate the offset field, so that
you have 8 bytes per struct (instead of 16).

>>
>> Do we want to require that the server MUST reply with enough extents to
>> sum up to the length of the client's request, or are we permitting a
>> short reply?
> 
> While the "GET LBA STATUS" command in SCSI allows partial reply, I
> believe it'd better to keep things simple and require that the server
> must either return a list of extents that covers the whole requested
> range, or an error.

Make sure you specify whether ranges are allowed to overlap, or must be
distinct (I prefer the latter), and whether they must appear in sorted
order (which I also prefer) - once you mandate ordered and
non-overlapping coverage, client-side validation that the server's
answer makes sense is easier (remember, we want the client to detect
man-in-the-middle corruption of the server's reply).


> Actually, for this command I treat 'command flags' field not as a set of
> flags, but rather as a plain number representing required mode of
> operation. Probably, not a good idea as it doesn't match the rest of the
> commands.
> 
> I went this way because I didn't want to allow clients to request
> several modes simultaneously (e.g. provisioning + dirtiness) in the same
> request. This makes server side implementation harder.
> 
> I think I'll just switch to bits to match the rest of the commands and
> will add a note, that server should return EINVAL in case several modes
> are requested simultaneously.

But you don't need two bits.  Just a single bit will do (off for
provisioning mode, on for dirty mode).  So there are no conflicting
modes that can be simultaneously requested, at least in the current
definition of a single valid flag bit.  (Then again, I did make a
suggestion about additional bits, useful only during provisioning mode,
that might be used to state that the client is okay if the server
coalesces extents that differ only in allocation or only in zeroed
content - if we add that bit or two bits, it would be an error to use it
while requesting dirty mode).


>> then we can express four states:
>>
>> 0x0 - LBA extent not present, client MUST NOT make assumptions about
>> contents, and reads should not be attempted
>> 0x1 - LBA extent allocated, reads will succeed but no guarantee on contents
>> 0x2 - LBA extent not present, but client can treat the extent as zeroes
>> and reads will succeed
>> 0x3 - LBA extent present, client can treat the extent as zeroes and
>> reads will succeed
> 
> I'm not sure that clients need this level of details. From client's POV
> 0x2 and 0x3 are the same.

No, if the client is trying to EXACTLY copy sparseness, then 0x2 and 0x3
differ on whether the client will punch a hole vs. explicitly allocate
zeroes.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]