Eric, (sic) >>> +During transmission phase, several operations are constrained by the >>> +export size sent in reply to `NBD_OPT_EXPORT_NAME` or `NBD_OPT_GO` >> >> or NBD_OPT_INFO? > > Not really - while NBD_OPT_INFO followed by NBD_OPT_GO _should_ send the > same export size, it is feasible that an intermediate NBD_OPT_ could > cause the server to advertise a different size; and the transmission > phase is bounded solely by the final advertised export size (ie. only > the two commands that can kick off transmission phase), not by earlier > advertisements. ok >>> , as >>> +well as by three block sizes defined here (minimum, preferred, and >>> +maximum). During the handshake phase, a client MAY announce its >>> +intention to honor server block sizes via the experimental >>> +`BLOCK_SIZE` extension; see below. A client that is worried about >>> +block size constraints SHOULD use `NBD_OPT_GO` rather than >>> +`NBD_OPT_EXPORT_NAME`. A server SHOULD advertise the block size >>> +contraints during handshake phase via the experimental `INFO` >>> +extension; see below. >> >> I agree, but this is a bit circular, given this is an experimental >> extension and thus there is no RFC2119 obligation to implement it. >> I think we should leave this in though as a reminded for us to >> deal with it when incorporated into the main standard. > > We're getting closer. I've already posted one version of qemu patches > for support for NBD_OPT_INFO, and I've been keeping them up-to-date with > my tweaking here in this patch churn; I'll post an updated working > implementation once we think we have the wording correct. And I will do gonbdserver. As per the other thread, I'm going to pull these off into a branch but will leave things until you've finished your open heart surgery. ... >>> A server that does advertise block sizes MUST be >>> +prepared for clients that ignore the advertisement, by, at the very >>> +least, sending appropriate errors, and MUST NOT corrupt data in that >>> +case. >> >> If we permit servers not avoid entering transmission mode with >> clients that don't respect block sizes, and the server also >> advertises block sizes, then if the client uses offsets / lengths >> that don't respect the block size rules, this is violation of the >> protocol and servers are already permitted to disconnect. The >> problem is thus with clients that don't claim they will respect >> block sizes. >> >> I think rather we should say: >> >> A server that advertises block sizes and [enters transmission >> mode with a client that does not advertise support for block >> sizes] MUST cleanly error commands that fall outside those blocksize >> parameters and not corrupt data. >> >> bit in [] optional I suppose. > > I'm wondering if there's some way to also cover a server that is willing > to advertise block sizes, but allows a client to enter transmission > phase with NBD_OPT_EXPORT_NAME, also falls in this category. I'm not sure it matters how it gets (or attempts to get) into transmission phase. If it got into transmission phase via NBD_OPT_EXPORT_NAME it could have obtained the block sizes via NBD_OPT_INFO or carrier pigeon. I think the simplest formulation is probably: A server that advertises block sizes MUST cleanly error commands that fall outside those blocksize parameters and not corrupt data. That appears to cover every case, given if the server does not advertise block sizes, the minimum is one and the maximum is infinity (subject to DDOS) so a request can't fall outside those bounds. >>> >>> +Where a transmission request can have a non-zero *offset* and/or >>> +*length* (such as `NBD_CMD_READ`, `NBD_CMD_WRITE`, or `NBD_CMD_TRIM`), >>> +the client MUST ensure that *offset* and *length* are integer >>> +multiples of any advertised minimum block size, and SHOULD use integer >>> +multiples of any advertised preferred block size where possible. For >>> +those requests, the client SHOULD NOT use a *length* of zero, >> >> I thought you wanted to ditch the zero length thing? Not sure what >> it adds here given your example re trim of length 0. 0 is an integer >> multiple of anything. > > In v2, it was MUST; here, I'm relaxing it to SHOULD (it is no longer a > protocol violation, but not useful, and may cause problems if we later > special-case the meaning of 0 length). Maybe add a phrase "unless > specific semantics are documented for a *length* of 0, the client SHOULD > NOT use a *length* of zero"? I don't understand why you need to bother with that here at all. It's documented alongside the commands themselves. Here you should be only saying "integer multiple" I think, which includes zero. You aren't (here) saying "any integer multiple is acceptable", you are saying "things that are not an integer multiple are not acceptable". >> Could we not just use one of the client flags? We have 32 bits of >> them (as opposed to 16 server flags). It's right at the start >> of the negotiation, and we simply need to refer to whether >> NBD_FLAG_C_BLOCKSIZE is set. It can be in the top 16 bits if we >> don't want to use a flag which can match a server flag. > > No: we already document "the server MUST drop the connection if the > client sets an unknown flag, or a flag that does not match something > advertised by the server." The only way for the client to use a client > flag rather than an NBD_OPT_ is for the server to first burn a global > flag, because otherwise, a new client setting the flag but talking to an > older server would never get past flag negotiation and have no good > error message to show for it. Whereas with NBD_OPT_, the client is at > least guaranteed a sane NBD_REP_ERR_UNSUP; and we have a lot more room > for adding new NBD_OPT_ (2^16) compared to adding new flags (2^5 global, > 2^6 client). Arrggh yes. So either we burn a global flag or we use an NBD_OPT, and add statefulness in the receipt of NBD options (i.e. the server now has to remember whether it's seen this option). Neither of these are especially pleasant. I think I'd prefer burning the flag :-( Wouter? -- Alex Bligh
Attachment:
signature.asc
Description: Message signed with OpenPGP using GPGMail