Re: Allowing > 32 bit lengths for NBD_CMD_TRIM, NBD_CMD_WRITE_ZEROES
On Tue, Oct 02, 2018 at 01:53:30PM +0200, Wouter Verhelst wrote:
> 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.
This is fine, was just wondering.
> > - 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 :-)
Understood, good points.
> > 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.
Yeah I would definitely try to avoid "structured". How about
"extended" request? Or even name them according to the size in bytes:
the current request would be "request28", the new request format would
be "request46", assuming I've added those fields up correctly.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
Reply to: