search for: nbd_internal_errno_of_nbd_error

On Mon, Jun 17, 2019 at 07:07:55PM -0500, Eric Blake wrote: > + errno = nbd_internal_errno_of_nbd_error (error); > + set_error (errno, "server reported read failure at offset 0x%" PRIx64, > + offset); > + if (cmd->cb.fn.read (cmd->cb.opaque, cmd->data + (offset - cmd->offset), > + 0, offset, LIBNBD_READ_ERRO...

Rather than moving the connection to DEAD, we can just ignore further contexts to the existing command handle, and fail the overall command with the same errno as the failed callback. Eric Blake (2): states: Track cmd->error as errno, not wire value api: Recover from block status callback failure generator/generator | 5 ++- generator/states-reply-simple.c | 2 +-

...tured.c b/generator/states-reply-structured.c index 9dcc898..f1d421d 100644 --- a/generator/states-reply-structured.c +++ b/generator/states-reply-structured.c @@ -228,13 +228,16 @@ type = be16toh (h->sbuf.sr.structured_reply.type); assert (cmd); /* guaranteed by CHECK */ + error = nbd_internal_errno_of_nbd_error (error); /* The spec requires the server to send a non-zero error */ if (error == NBD_SUCCESS) debug (h, "server forgot to set error; using EINVAL"); error = nbd_internal_errno_of_nbd_error (error ? error : EINVAL); - /* Sanity check that any error offset is in...

...(h->sbuf.sr.structured_reply.flags); handle = be64toh (h->sbuf.sr.structured_reply.handle); error = be32toh (h->sbuf.sr.payload.error.error.error); type = be16toh (h->sbuf.sr.structured_reply.type); @@ -264,10 +262,7 @@ if (cmd->error == 0) cmd->error = nbd_internal_errno_of_nbd_error (error); - if (flags & NBD_REPLY_FLAG_DONE) - SET_NEXT_STATE (%^FINISH_COMMAND); - else - SET_NEXT_STATE (%.READY); + SET_NEXT_STATE(%FINISH); } return 0; @@ -334,24 +329,15 @@ return 0; REPLY.STRUCTURED_REPLY.RECV_OFFSET_DATA_DATA: - uint16_t flags; - switc...

.... */ - return nbd_internal_run (conn->h, conn, cmd_issue); + return 0; } diff --git a/lib/internal.h b/lib/internal.h index 3f2b729..67bd52a 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -265,6 +265,12 @@ extern void nbd_internal_set_last_error (int errnum, char *error); extern int nbd_internal_errno_of_nbd_error (uint32_t error); extern const char *nbd_internal_name_of_nbd_cmd (uint16_t type); +/* rw.c */ +extern struct command_in_flight *command_common (struct nbd_connection *conn, + uint16_t flags, uint16_t type, +...

...((errnum), _errp); \ } while (0) +/* flags.c */ +extern int nbd_internal_set_size_and_flags (struct nbd_handle *h, + uint64_t exportsize, + uint16_t eflags); + /* protocol.c */ extern int nbd_internal_errno_of_nbd_error (uint32_t error); extern const char *nbd_internal_name_of_nbd_cmd (uint16_t type); -- 2.21.0

...18 @@ offset, cmd->offset, cmd->count); return -1; } + if (cmd->cb.fn.read) { + /* Different from successful reads: if the callback clears + * errno but still fails, then use the server's error below. + */ + errno = nbd_internal_errno_of_nbd_error (error); + set_error (errno, "server reported read failure at offset 0x%" PRIx64, + offset); + if (cmd->cb.fn.read (cmd->cb.opaque, cmd->data + (offset - cmd->offset), + 0, offset, LIBNBD_READ_ERROR) == -1) +...

...enum state state); +extern bool nbd_internal_is_state_ready (enum state state); +extern bool nbd_internal_is_state_processing (enum state state); +extern bool nbd_internal_is_state_dead (enum state state); +extern bool nbd_internal_is_state_closed (enum state state); + /* protocol.c */ extern int nbd_internal_errno_of_nbd_error (uint32_t error); extern const char *nbd_internal_name_of_nbd_cmd (uint16_t type); diff --git a/lib/is-state.c b/lib/is-state.c index 5ed2ee9..55d103b 100644 --- a/lib/is-state.c +++ b/lib/is-state.c @@ -26,11 +26,12 @@ #include "internal.h" -/* NB: is_locked = false, may_set_error...

...TRUCTURED_REPLY.RECV_ERROR_TAIL: struct command_in_flight *cmd; uint16_t flags; uint64_t handle; uint32_t error; + uint64_t offset; switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return -1; @@ -183,6 +231,20 @@ if (cmd->error == 0) cmd->error = nbd_internal_errno_of_nbd_error (error); + /* Sanity check that any error offset is in range */ + if (error == NBD_REPLY_TYPE_ERROR_OFFSET) { + offset = be64toh (h->sbuf.offset); + if (offset < cmd->offset || offset >= cmd->offset + cmd->count) { + SET_NEXT_STATE (%.DEAD); + set_er...

...+int +nbd_unlocked_can_fast_zero (struct nbd_handle *h) +{ + return get_flag (h, NBD_FLAG_SEND_FAST_ZERO); +} + int nbd_unlocked_can_df (struct nbd_handle *h) { diff --git a/lib/protocol.c b/lib/protocol.c index 6087887..acee203 100644 --- a/lib/protocol.c +++ b/lib/protocol.c @@ -36,6 +36,7 @@ nbd_internal_errno_of_nbd_error (uint32_t error) case NBD_EINVAL: return EINVAL; case NBD_ENOSPC: return ENOSPC; case NBD_EOVERFLOW: return EOVERFLOW; + case NBD_ENOTSUP: return ENOTSUP; case NBD_ESHUTDOWN: return ESHUTDOWN; default: return EINVAL; } diff --git a/lib/rw.c b/lib/rw.c index d427681..adb6bc2 10064...

.... */ - return nbd_internal_run (conn->h, conn, cmd_issue); + return 0; } diff --git a/lib/internal.h b/lib/internal.h index 1f742da..de9b8bc 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -265,6 +265,13 @@ extern void nbd_internal_set_last_error (int errnum, char *error); extern int nbd_internal_errno_of_nbd_error (uint32_t error); extern const char *nbd_internal_name_of_nbd_cmd (uint16_t type); +/* rw.c */ +extern int64_t nbd_internal_command_common (struct nbd_connection *conn, + uint16_t flags, uint16_t type, + uint64_...

...#define NBD_ENOMEM 12 #define NBD_EINVAL 22 #define NBD_ENOSPC 28 +#define NBD_EOVERFLOW 75 #define NBD_ESHUTDOWN 108 #endif /* NBD_PROTOCOL_H */ diff --git a/lib/protocol.c b/lib/protocol.c index d3ac0b4..6087887 100644 --- a/lib/protocol.c +++ b/lib/protocol.c @@ -35,6 +35,7 @@ nbd_internal_errno_of_nbd_error (uint32_t error) case NBD_ENOMEM: return ENOMEM; case NBD_EINVAL: return EINVAL; case NBD_ENOSPC: return ENOSPC; + case NBD_EOVERFLOW: return EOVERFLOW; case NBD_ESHUTDOWN: return ESHUTDOWN; default: return EINVAL; } diff --git a/lib/rw.c b/lib/rw.c index 24dbc4e..2dc60de 100644...

Since v1: - rebase to applied patches - split out support for Int in callbacks - sort of test that callbacks work in OCaml (see comment in patch 5) - rename API to nbd_pread_structured - expose error as explicit parameter to callback Eric Blake (5): generator: Allow Int in callbacks states: Wire in a read callback states: Add nbd_pread_structured API states: Add tests for

Per recent discussion here: https://www.redhat.com/archives/libguestfs/2019-May/thread.html#00175

...V_ERROR: > > resync: > /* Favor the error packet's errno over RESYNC's EPROTO. */ > - error = be32toh (h->sbuf.sr.payload.error.error.error); > + error = be32toh (h->sbuf.reply.payload.error.error.error); > if (cmd->error == 0) > cmd->error = nbd_internal_errno_of_nbd_error (error); > h->rbuf = NULL; > - h->rlen = length - MIN (length, sizeof h->sbuf.sr.payload.error.error); > + h->rlen = length - MIN (length, sizeof h->sbuf.reply.payload.error.error); > SET_NEXT_STATE (%RESYNC); > return 0; > > @@ -195,15 +195,15 @@ RE...

I've mentioned this topic before (in fact, the idea of adding NBD_CMD_FLAG_DF was first mentioned at [1]), but finally finished enough of an implementation to feel confident in posting it. I'd still like to add something under examples/ that uses the new API to implement strict checking of a server's structured replies read implementation (ensure that a server never sends data after

...9 @@ extern bool nbd_internal_is_state_processing (enum state state); extern bool nbd_internal_is_state_dead (enum state state); extern bool nbd_internal_is_state_closed (enum state state); +/* opt.c */ +extern void nbd_internal_free_option (struct nbd_handle *h); + /* protocol.c */ extern int nbd_internal_errno_of_nbd_error (uint32_t error); extern const char *nbd_internal_name_of_nbd_cmd (uint16_t type); diff --git a/generator/API.ml b/generator/API.ml index c660960..52970d3 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -127,8 +127,12 @@ let extent_closure = { "nr_entries&...

On v1, we discussed whether cmds_to_issue needed to be a list, since it never had more than one element. I played with the idea of making it a list, and allowing the client to queue up new commands regardless of whether the state machine is currently in READY. I also polished up the tmp demo into a bit more full-fledged example file, worth including since it also let me discover a hard-to-hit race

This was v3 patch 2/22, reworked to address the confusion about how a structured reply header is read in two pieces before getting to the payload portion. I'm still working on rebasing the rest of my v3 series (patches 1, 3-22) from other comments given, but this seemed independent enough that it's worth posting now rather than holding it up for the rest of the series. Eric Blake (4):

I need to think about this patch series a bit more, but it does at least pass the tests. Rich.