Commit b31e7bac authored by Eric Blake's avatar Eric Blake
Browse files

states: Tolerate simple reply to structured read request

Another patch in the series of being more tolerant to various server
bugs.  Although a server is non-compliant for using a simple reply to
NBD_CMD_READ once structured commands are negotiated, it is just as
easy to read the packet and continue on after guaranteeing we report
an error.

In the process, I noticed that in the corner case of a server that
starts with a structured packet and follows with a simple reply, we
can no longer assume that the simple reply is the only receipt of
data, so we have to be more careful with assertions and increments to
data_seen.

As in earlier patches, the following temporary one-line tweak to
nbdkit will produce a server that demonstrates the scenario:

| diff --git i/server/protocol.c w/server/protocol.c
| index 2ac77055..ac359ca1 100644
| --- i/server/protocol.c
| +++ w/server/protocol.c
| @@ -738,7 +738,7 @@ protocol_recv_request_send_reply (void)
|     * us from sending human-readable error messages to the client, so
|     * we should reconsider this in future.
|     */
| -  if (conn->structured_replies &&
| +  if (conn->structured_replies && !error &&
|        (cmd == NBD_CMD_READ || cmd == NBD_CMD_BLOCK_STATUS)) {
|      if (!error) {
|        if (cmd == NBD_CMD_READ)

The following command line:

$ ./run /path/to/nbdkit -U - memory 1M --filter=error error-pread-rate=100% \
    --run 'nbdsh -u "$uri" -c "\
try:
  h.pread(1,0)
except nbd.Error as ex:
  print(ex.string)
h.flush()
print(h.get_size())
"'

demonstrates the following issue pre-patch:

nbdkit: memory.0: error: injecting EIO error into pread
nbd_pread: server sent unexpected simple reply for read
nbdsh: command line script failed: nbd_flush: invalid state: DEAD: the handle must be connected with the server: Invalid argument

and the nicer flow post-patch:

nbdkit: memory.0: error: injecting EIO error into pread
nbd_pread: read: command failed: Protocol error
1048576

where we report EPROTO rather than the server's EIO.
parent 4e57e2d9
Pipeline #616289691 failed with stage
in 9 minutes and 48 seconds
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment