[Libguestfs] [libnbd PATCH 3/3] states: Allow in-flight read while writing next command

[Eric Blake]

As already noted in our state machine, a client that batches up a
large read followed by large writes, coupled with a server that only
processes commands in order, can result in deadlock (the server won't
read more until we unblock its ability to write out its reply to our
first command; but we aren't willing to read until we are done writing
out our second command).  Break the deadlock by teaching the generator
that while we are in the middle of writing a command, we must remain
responsive to read_notify events; if the server has data for us to
read, we should consume that before jumping back into the middle of
our command issue (and consuming a reply can invalidate sbuf, so we
have to drop an assertion in PREPARE_WRITE_PAYLOAD).
---
 generator/generator              | 20 ++++++++++++++++++--
 generator/states-issue-command.c | 25 ++++++++++++++++++++++++-
 generator/states-reply.c         |  5 ++++-
 lib/internal.h                   |  1 +
 4 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/generator/generator b/generator/generator
index a4ad362..23b3cbf 100755
--- a/generator/generator
+++ b/generator/generator
@@ -634,7 +634,15 @@ and issue_command_state_machine = [
     default_state with
     name = "SEND_REQUEST";
     comment = "Sending a request to the remote server";
-    external_events = [ NotifyWrite, "" ];
+    external_events = [ NotifyWrite, "";
+                        NotifyRead, "PAUSE_SEND_REQUEST" ];
+  };
+
+  State {
+    default_state with
+    name = "PAUSE_SEND_REQUEST";
+    comment = "Interrupt send request to receive an earlier command's reply";
+    external_events = [];
   };

   State {
@@ -648,7 +656,15 @@ and issue_command_state_machine = [
     default_state with
     name = "SEND_WRITE_PAYLOAD";
     comment = "Sending the write payload to the remote server";
-    external_events = [ NotifyWrite, "" ];
+    external_events = [ NotifyWrite, "";
+                        NotifyRead, "PAUSE_WRITE_PAYLOAD" ];
+  };
+
+State {
+    default_state with
+    name = "PAUSE_WRITE_PAYLOAD";
+    comment = "Interrupt write payload to receive an earlier command's reply";
+    external_events = [];
   };

 State {
diff --git a/generator/states-issue-command.c b/generator/states-issue-command.c
index e24ea34..3a5980d 100644
--- a/generator/states-issue-command.c
+++ b/generator/states-issue-command.c
@@ -25,6 +25,15 @@
   assert (conn->cmds_to_issue != NULL);
   cmd = conn->cmds_to_issue;

+  /* Were we interrupted by reading a reply to an earlier command? */
+  if (conn->wlen) {
+    if (conn->in_write_payload)
+      SET_NEXT_STATE(%SEND_WRITE_PAYLOAD);
+    else
+      SET_NEXT_STATE(%SEND_REQUEST);
+    return 0;
+  }
+
   conn->sbuf.request.magic = htobe32 (NBD_REQUEST_MAGIC);
   conn->sbuf.request.flags = htobe16 (cmd->flags);
   conn->sbuf.request.type = htobe16 (cmd->type);
@@ -43,12 +52,18 @@
   }
   return 0;

+ ISSUE_COMMAND.PAUSE_SEND_REQUEST:
+  assert (conn->wlen);
+  assert (conn->cmds_to_issue != NULL);
+  conn->in_write_payload = false;
+  SET_NEXT_STATE (%^REPLY.START);
+  return 0;
+
  ISSUE_COMMAND.PREPARE_WRITE_PAYLOAD:
   struct command_in_flight *cmd;

   assert (conn->cmds_to_issue != NULL);
   cmd = conn->cmds_to_issue;
-  assert (cmd->handle == be64toh (conn->sbuf.request.handle));
   if (cmd->type == NBD_CMD_WRITE) {
     conn->wbuf = cmd->data;
     conn->wlen = cmd->count;
@@ -65,9 +80,17 @@
   }
   return 0;

+ ISSUE_COMMAND.PAUSE_WRITE_PAYLOAD:
+  assert (conn->wlen);
+  assert (conn->cmds_to_issue != NULL);
+  conn->in_write_payload = true;
+  SET_NEXT_STATE (%^REPLY.START);
+  return 0;
+
  ISSUE_COMMAND.FINISH:
   struct command_in_flight *cmd;

+  assert (!conn->wlen);
   assert (conn->cmds_to_issue != NULL);
   cmd = conn->cmds_to_issue;
   conn->cmds_to_issue = cmd->next;
diff --git a/generator/states-reply.c b/generator/states-reply.c
index 45362d4..6bb503a 100644
--- a/generator/states-reply.c
+++ b/generator/states-reply.c
@@ -118,7 +118,10 @@
   else
     conn->cmds_done = cmd;

-  SET_NEXT_STATE (%.READY);
+  if (conn->cmds_to_issue)
+    SET_NEXT_STATE (%^ISSUE_COMMAND.START);
+  else
+    SET_NEXT_STATE (%.READY);
   return 0;

 } /* END STATE MACHINE */
diff --git a/lib/internal.h b/lib/internal.h
index 3f2b729..466af9d 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -182,6 +182,7 @@ struct nbd_connection {
    * acknowledge them.
    */
   struct command_in_flight *cmds_to_issue, *cmds_in_flight, *cmds_done;
+  bool in_write_payload;
 };

 struct meta_context {
-- 
2.20.1