Re: [Nbd] proposal

[Wouter Verhelst <w@...112...>]

On Thu, Sep 15, 2011 at 04:53:52PM +0100, Alex Bligh wrote:
> 
> > That sounds like something that could be done in the negotiation phase,
> > during the option haggling. I don't think I'd like to see this
> > afterwards.
> 
> +1. Not least because the client by then is the kernel.
> 
> >> - NBD_ELABORATE_ON_ERROR
> >>  if a command fails, an error code is returned. afaik this is an errno
> >> value. sometimes you want to elaborate on this. for example a
> >> server-device which detected that the datastructures got invalid or so
> >> me return EINVALID (or whatever) and in response to this command a
> >> textual description of what is wrong: "datastore corrupt, run fsck"
> >
> > Yes, there're definite problems with the current error handling.
> 
> +1 :-)
> 
> In particular error handling of reads is fundamentally broken as
> per a previous message to this list.
> 
> > I
> > didn't realize it at the time when I suggested it, but the absolute
> > value of errno is very ill defined; POSIX only defines the symbolic
> > names, nothing more, and on Linux the actual value can even be different
> > from one architecture to another.
> 
> Wow, didn't know that.

Yeah, well, I found out the hard way :-)

> > I've been thinking of creating an NBD-specific list of error values, so
> > that a client can have some more useful information.
> >
> > Adding textual info could also be an interesting idea, but the only
> > problem I see with that is that it would entail a change of the
> > protocol. Currently, an error returns only the nbd_reply data, nothing
> > more; if we want to add textual data to that, it would require adding a
> > length field, followed by an amount of textual data to the reply. This
> > is doable, I suppose, but would change the protocol incompatibly.
> 
> I think it might need to change anyway (see reads).
> 
> My problem with ELABORATE_ON_ERROR is "on which error?". An error
> does not indicate the end of the protocol session, and a whole
> string of errors could be produced asynchronously. Does the server
> have to store them up just in case the client wants later elaboration?
> If so, for how long?
> 
> The only sensible thing to do is to send the error with the response.
> 
> The errors also should be machine parsable. Perhaps borrow someone
> else's well-defined error codes which are like POSIX. NFS perhaps?

I don't think we need to go that far. The errno(3) manpage lists a *lot*
of error codes, which we really don't need. And remember that if you add
such a thing, the nbd kernel module needs to be able to decode it, too.
Also, we sometimes want to manufacture errno codes to deal with
malicious or buggy clients sending invalid requests.

I'm thinking that with an enum like this, we should be fine:

enum {
        NBD_ERR_OK = 0,    /* No error */
        NBD_ERR_INVAL = 1, /* Invalid request */
        NBD_ERR_IO = 2,    /* Reading from or writing to the backend storage
                              failed */
        NBD_ERR_NOSPC = 3, /* No space left on device while trying to write to
                              backend storage */
        NBD_ERR_PERM = 4,  /* Opening the device was not allowed, for any
                              reason */
        NBD_ERR_RO = 5,    /* Write attempted on read-only device */
        NBD_ERR_RANGE = 6, /* A request was made outside the size of the NBD
                              device */
};

INVAL would mean the client sent something wrong to the server; nbd.ko
should return EINVAL.

IO would occur when nbd-server receives EIO, or EPIPE, or EINTR, or
EFAULT, or some such. Rationale: whether a read or write fails because
the server has a bug causing it to write outside his accessible address
space (EFAULT), or because the write was interrupted and cannot be
restarted (EINTR) shouldn't matter to the client; the write failed, and
that's an I/O error as far as the client is concerned (the server log
should clarify, through the use of strerror(), what happened). nbd.ko
should return EIO.

NOSPC would be sent when a write() operation returns ENOSPC, and nbd.ko
should return the same.

PERM would occur when a client tries to open a device, but the
authorization file does not allow this, or nbd-server can't open the
backend file due to file access permissions, or some such. nbd.ko should
return EACCESS, if anything (this error code would probably only be used
during negotiation. Currently we don't send a description of what went
wrong to the client when we don't want to provide any service, but
that's probably not a bad idea anyway)

RO should speak for itself.

RANGE is something that could probably be part of INVAL, but there's a
separate errno ERANGE, so it could be helpful to keep that.

Thoughts, additions?

-- 
The volume of a pizza of thickness a and radius z can be described by
the following formula:

pi zz a