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

[Pavel Borzenkov <pborzenkov@virtuozzo.com>]

On Thu, Mar 24, 2016 at 09:04:07AM -0600, Eric Blake wrote:
> 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?

On second thought, not sending offsets look better. Firstly, they are
really not required for client to process the reply. Secondly, it also
implies that the regions are distinct and in sorted order and makes it
impossible to do otherwise.

-- 
Pavel

> 
> > 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
>