Re: [Nbd] error messaging

[Goswin von Brederlow <goswin-v-b@...186...>]

On Tue, Apr 15, 2014 at 07:28:27AM +0200, Wouter Verhelst wrote:
> Hi,
> 
> I've been working on a way to reimplement the core of nbd-server, in the
> io_transaction branch. It's not moving along very fast, but steadily so.
> 
> Today, I was rethinking things a bit again, and came to the conclusion
> that there's a problem in the error messaging from server to client in
> case of a read error.
> 
> The protocol currently wants the server to send the reply header before
> the data. If there's any error, the error should be signalled through
> the error field in the reply header.
> 
> This implies that the server must read the full data from disk to memory
> before sending anything to the client. If it were to use sendfile() or
> similar things that don't use a memory buffer and an error occurred
> while reading from disk, then the reply header will already have been
> sent and there's nothing the server can do anymore to signal this
> problem.
> 
> This could be fixed, after proper negotiation, by sticking a second
> reply header after read requests in which the server can signal error
> messages. The sequence of data sent from server to client on a read
> request would then look as follows:
> 
> reply header -- if the request contained any error (i.e., request was
> 		out of bounds for the client), then the error field is
> 		set here and no data is sent.
> data -- if any error occurred while reading from the disk, data is
> 		padded with zeroes.
> 2nd reply header -- if an error occured while reading from disk, the
> 		error field is set here.
> 
> This would be an incompatible change, so it should only be enabled if
> the client signalled that it supports this.
> 
> Thoughts?

I thought the right way to behave then would be to close the socket
mid stream. The data for the request would be incomplete and the
client sees an error. Not nice but currently the only way.

The thought I had about your idea is that it increases overhead. Maybe
this could be kept to a minimum. The 2nd reply header doesn't have to
be bit. All we want to indicate is that the read was partial for
whatever reason and only the first X bytes of the reply are valid.

reply header -- if the request contained any error (i.e., request was
                out of bounds for the client), then the error field is
                set here and no data is sent.
data -- if any error occurred while reading from the disk, data is
                padded with zeroes.
size -- amount of data that is valid

This would only increase the overhead by 4 bytes.

My question though is: Is this worth it? Doesn't splice replace
sendfile in a way that this becomes unnecessary?

With splice you first splice the data into a pipe. If there are read
errors you get them there already. Once all the data is all stored in
the pipe you send the reply header and the splice the pipe into the
socket. Unlike sendfile this allows to handle multiple requests in
parallel.

I know splice is linux specific but don't other OSes have something
similar nowadays?


If we are talking about changing the protocol I would also like to
support resize. Resize can happen in one of two ways:

1) The client sends some form of resize request causing the server to
rescan the device size (or actually do the resize if it can) and reply
with the new geometry for the device. This only works for a single
client though.

2) The server gets told to resize, by a client (see 1) or some other
mechanism. Now it has to inform all clients about the change. But
there is no provision in the protocol for the server to just send out
a reply without request.

This could be solved by making the protocol bidirectional. Or by
taking advantage of the fact that requests can be asyncronous. Why not
have each client send a "tell me about resizes" request on connect.
The request would be stored on the server side until such a time when
a resize actually happens. Then it sends a reply and the client sends
a new request. That way the server always has a request pending to
report changes.

MfG
	Goswin