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

[Eric Blake]

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