[Libguestfs] [libnbd PATCH 1/3] states: Refactor where interrupted reply state is stored

There can be at most one active reply at a time. Storing a resume state per command is wasteful compared to storing a single resume state in the overall handle. Furthermore, if we want to be able to skip bogus server replies to an unexpected cookie, we need a way to resume reading bytes off the wire even when there is no matching h->reply_cmd for the current cookie. --- lib/internal.h | 4 ++-- generator/states-reply.c | 20 +++++++++++--------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/lib/internal.h b/lib/internal.h index 8dbe2e4..0a61b15 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -303,8 +303,9 @@ struct nbd_handle { /* length (cmds_to_issue) + length (cmds_in_flight). */ int in_flight; - /* Current command during a REPLY cycle */ + /* Current command and POLLIN resumption point during a REPLY cycle */ struct command *reply_cmd; + enum state reply_state; bool disconnect_request; /* True if we've queued NBD_CMD_DISC */ bool tls_shut_writes; /* Used by lib/crypto.c to track disconnect. */ @@ -352,7 +353,6 @@ struct command { uint32_t count; void *data; /* Buffer for read/write */ struct command_cb cb; - enum state state; /* State to resume with on next POLLIN */ bool initialized; /* For read, true if getting a hole may skip memset */ uint32_t data_seen; /* For read, cumulative size of data chunks seen */ uint32_t error; /* Local errno value */ diff --git a/generator/states-reply.c b/generator/states-reply.c index bff6f4b..1a3a20a 100644 --- a/generator/states-reply.c +++ b/generator/states-reply.c @@ -1,5 +1,5 @@ /* nbd client library in userspace: state machine - * Copyright (C) 2013-2019 Red Hat Inc. + * Copyright (C) 2013-2022 Red Hat Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -25,26 +25,28 @@ * until POLLIN, we instead track where in the state machine we left * off, then return to READY to actually block. Then, on entry to * REPLY.START, we can tell if this is the start of a new reply (rlen - * is 0, stay put), a continuation of the preamble (reply_cmd is NULL, - * resume with RECV_REPLY), or a continuation from any other location - * (reply_cmd contains the state to jump to). + * is 0, stay put), a continuation of the preamble (reply_state is + * STATE_START, resume with RECV_REPLY), or a continuation from any + * other location (reply_state contains the state to jump to). */ static void save_reply_state (struct nbd_handle *h) { - assert (h->reply_cmd); assert (h->rlen); - h->reply_cmd->state = get_next_state (h); + assert (h->reply_state == STATE_START); + h->reply_state = get_next_state (h); + assert (h->reply_state != STATE_START); } STATE_MACHINE { REPLY.START: /* If rlen is non-zero, we are resuming an earlier reply cycle. */ if (h->rlen > 0) { - if (h->reply_cmd) { - assert (nbd_internal_is_state_processing (h->reply_cmd->state)); - SET_NEXT_STATE (h->reply_cmd->state); + if (h->reply_state != STATE_START) { + assert (nbd_internal_is_state_processing (h->reply_state)); + SET_NEXT_STATE (h->reply_state); + h->reply_state = STATE_START; } else SET_NEXT_STATE (%RECV_REPLY); -- 2.37.1