Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

[Wouter Verhelst]

On Tue, Apr 05, 2016 at 08:14:01AM -0600, Eric Blake wrote:
> On 04/05/2016 03:24 AM, Markus Pargmann wrote:
> 
> >> +        requested.
> >> +
> >> +    The client SHOULD NOT read from an area that has both
> >> +    `NBD_STATE_HOLE` set and `NBD_STATE_ZERO` clear.
> > 
> > Why not? If we don't execute CMD_BLOCK_STATUS we wouldn't know about the
> > situation and would simply read directly. To fulfill this statement we
> > would need to receive the block status before every read operation.
> 
> Because we already state that for NBD_CMD_TRIM, the client SHOULD NOT
> read an area where a trim was requested without an intervening write,

Actually, we state that a client SHOULD NOT *make any assumption* about
the state (emphasis added). There's a difference.

If the client wants to write, there is no problem (but he'll probably
get garbage).

[...]
> > Also something that is kind of missing from the document so far is
> > concurrency with other NBD clients. Certainly most users do not use NBD
> > for concurrent access to the backend storage. But for example the
> > sentence above ignores the fact that another client may work on the
> > backend and that the state may change after some time so that it may
> > still be necessary to read from an area with NBD_STATE_HOLE and
> > NBD_STATE_ZERO set.
> 
> That's missing from NBD in general, and I don't think this is the patch
> to add it.  We already have concurrency with self issues, because NBD
> server can handle requests out of order (within the bounds of FLUSH and
> FUA modifiers).

I don't think NBD *needs* to add much in the way of concurrency, other
than write barriers etc; but having an explicit message "some other
process just modified offset X length Y" seems out of scope for NBD.

> > Also it is uncertain if these status bits may change over time through
> > reorganization of backend storage, for example holes may be removed in
> > the backend and so on. Is it safe to cache this stuff?
> 
> If the client is the only thing modifying the drive, maybe we want to
> make that additional constraint on the server.  But how best to word it,
> or is it too tight of a specification?

I think it's perfectly fine for the protocol to assume in general that
the client is the only writer, and that no other writers are
concurrently accessing the same device. Anything else would make the
protocol much more complex; and one of the features of NBD is, I think,
the fact that the protocol is not *too* complex.

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
       people in the world who think they really understand all of its rules,
       and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12