The server probably shouldn't disconnect in case of write errors according to the protocol?

[gaborigloi]

The NBD protocol says about NBD_CMD_WRITE:

A write request. Length and offset define the location and amount of data to be written. The client MUST follow the request header with length number of bytes to be written to the device. The server MUST write the data to disk, and then send the reply message. The server MAY send the reply message before the data has reached permanent storage. If an error occurs, the server MUST set the appropriate error code in the error field.

And about disconnecting during the transmission phase:

Either side MAY initiate a hard disconnect if it detects a violation by the other party of a mandatory condition within this document.

and:

The client and the server MUST NOT initiate any form of disconnect other than in one of the above circumstances.

And about NBD_FLAG_READ_ONLY:

The server MAY set this flag to indicate to the client that the export is read-only (exports might be read-only in a manner undetectable to the server, for instance because of permissions). If this flag is set, the server MUST error subsequent write operations to the export."

Thus it seems that the server should not disconnect in case of NBD_CMD_WRITE errors, but should continue processing the client's requests.

[gaborigloi]

So the fix for this would be an extensions of my fix for #41 (#43) , I probably shouldn't have followed the pattern in the handler for NBD_CMD_READ (only continuing the loop when everything is OK), but I should have continued the loop unconditionally.

[gaborigloi]

After the discussions on #106 and #107 , maybe we should close this:

Although in cases like #106 when we do not terminate the connection there is always a risk that the client ignores the error and only checks for it after sending all the write data, and then when the NBD server tries to read the next request, it might interpret the data the client wants to write as an NBD request, and as a result might do completely random things, which is bad. So maybe this PR is not much worse than #106 It really depends on how NBD clients are implemented, and what the protocol says about this scenario.

[gaborigloi]

Maybe we should clarify this on the NBD mailing list though?

[mseri]

Why not?

[gaborigloi]

Thinking about this some more, I think the correct behaviour in case of a read-only export, when the client sends the write request, is to read & discard the data, and then respond with the error message:

NBD_CMD_WRITE (1)

A write request. Length and offset define the location and amount of data to be written. The client MUST follow the request header with length number of bytes to be written to the device. The client SHOULD NOT request a write length of 0; the behavior of a server on such a request is unspecified although the server SHOULD NOT disconnect.

The server MUST write the data to disk, and then send the reply message. The server MAY send the reply message before the data has reached permanent storage, unless NBD_CMD_FLAG_FUA is in use.

If an error occurs, the server MUST set the appropriate error code in the error field.

So we know that "the client MUST follow the request header with length number of bytes to be written to the device", so we can safely read & discard this data.

[gaborigloi]

Also, the server will not do random things if it gets out of sync with the client, because there is a 32-bit magic value at the beginning of every request, and the probability that some other random data will match this magic value is very low. If the magic value is incorrect, the server will simply stop serving the client.