[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Allowing > 32 bit lengths for NBD_CMD_TRIM, NBD_CMD_WRITE_ZEROES



On Tue, Oct 02, 2018 at 12:12:45PM +0100, Richard W.M. Jones wrote:
> A few questions about the proposal:
> 
> - Why must the client request NBD_INFO_BLOCK_SIZE?

The idea of that was so that a client can't go berserk with a 64-bit
request even though the space is there. Making block sizes a requirement
would avoid that issue.

It's a minor thing though, and if it feels like a bad idea I'm happy to
drop it.

> - Why must the client not mix simple & structured requests?  It seems
>   like a fairy arbitrary restriction, in that it doesn't make the
>   client or server any easier to implement,

There were two reasons.

First, I can see that at some point in the future, we might want to drop
the older format. A newly-written server that doesn't (want to)
implement the older format would need to be able to produce an error
message to a client when it knows after negotiation that it might
receive messages in a format that it can't parse. I considered having a
response to NBD_OPT_STRUCTURED_REQUEST that would state that it *only*
supports the new format, but decided that that seems a bit excessive and
that it's easier to just require the new format everywhere; a server
that doesn't support the old format could then just error out on
NBD_OPT_GO if the new message format hasn't been negotiated.

Second, I actually think that it *would* make the server slightly easier
to implement. The bit of the server that reads requests off the wire
could then just the number of bytes of the standard header off the wire,
figure out what the request size is, read additional bytes that it
hasn't read yet and store them, and then hand off the request to
whatever bit of the server implementation would handle it. Allowing for
the possibility of differently-sized requests would add a few
conditionals in that code, whereas a server that implements both
request formats under the condition that only one of both is allowed
could store the size of the header in a per-connection state variable or
something along those lines.

But maybe I'm just overthinking things here :-)

>   nor does it make protocol
>   analysis tools any simpler.  In fact it makes things slightly more
>   complicated because it implies that there's now a hidden mode for
>   each connection.

Yeah, but as soon as the analysis tool has seen one message it would be
able to infer from that what the mode is; I don't think that's a huge
problem.

> Also I'm not sure about the name "structured" request.  Why not
> "large" request (and call the current one "small" request)?  The name
> "structured" also implies there is some connection with this feature
> and the Structured Replies feature, but AFAIK there is no connection.

There is one, in that I reused the magic number for structured replies,
but that's minor (and probably not even a good idea at all, too; we use
different magic numbers for normal requests and replies too, so, hrm)

I was trying to come up with a good name and didn't want to repeat the
"oldstyle"/"newstyle"/"fixed newstyle" mistake that I made with the
names for the negotiation protocol phases. "large" and "small" seem
slightly better in that respect, except that if we then add another
field later it becomes... "extra large"? Or something. Not sure about
that one.

I'm definitely not happy with "structured" myself either for the reasons
you quote (and others), but it feels a bit like the lesser evil to me at
this point in time.

-- 
Could you people please use IRC like normal people?!?

  -- Amaya Rodrigo Sastre, trying to quiet down the buzz in the DebConf 2008
     Hacklab


Reply to: