[Libguestfs] [libnbd PATCH 2/3] states: Split ISSUE_COMMAND.SEND_REQUEST

[Richard W.M. Jones]

On Tue, May 21, 2019 at 10:09:29AM -0500, Eric Blake wrote:
> In order to handle reading an in-flight response while in the middle
> of sending a second command, we'll need a way to jump back into the
> middle of a command being sent. This is easier if the state that sets
> wbuf is distinct from the state that reads into wbuf, and if we don't
> move the command to the in-flight queue until after the writes finish.
> ---
>  generator/generator              | 14 ++++++++++
>  generator/states-issue-command.c | 45 ++++++++++++++++++++------------
>  2 files changed, 42 insertions(+), 17 deletions(-)
> 
> diff --git a/generator/generator b/generator/generator
> index a1bf41d..a4ad362 100755
> --- a/generator/generator
> +++ b/generator/generator
> @@ -637,12 +637,26 @@ and issue_command_state_machine = [
>      external_events = [ NotifyWrite, "" ];
>    };
> 
> +  State {
> +    default_state with
> +    name = "PREPARE_WRITE_PAYLOAD";
> +    comment = "Prepare the write payload to send to the remote server";
> +    external_events = [];
> +  };
> +
>    State {
>      default_state with
>      name = "SEND_WRITE_PAYLOAD";
>      comment = "Sending the write payload to the remote server";
>      external_events = [ NotifyWrite, "" ];
>    };
> +
> +State {
> +    default_state with
> +    name = "FINISH";
> +    comment = "Finish issuing a command";
> +    external_events = [];
> +  };
>  ]
> 
>  (* Receiving a reply from the server. *)
> diff --git a/generator/states-issue-command.c b/generator/states-issue-command.c
> index a57f40f..e24ea34 100644
> --- a/generator/states-issue-command.c
> +++ b/generator/states-issue-command.c
> @@ -24,9 +24,6 @@
> 
>    assert (conn->cmds_to_issue != NULL);
>    cmd = conn->cmds_to_issue;
> -  conn->cmds_to_issue = cmd->next;
> -  cmd->next = conn->cmds_in_flight;
> -  conn->cmds_in_flight = cmd;
> 
>    conn->sbuf.request.magic = htobe32 (NBD_REQUEST_MAGIC);
>    conn->sbuf.request.flags = htobe16 (cmd->flags);
> @@ -40,29 +37,43 @@
>    return 0;
> 
>   ISSUE_COMMAND.SEND_REQUEST:
> -  struct command_in_flight *cmd;
> -
>    switch (send_from_wbuf (conn)) {
>    case -1: SET_NEXT_STATE (%.DEAD); return -1;
> -  case 0:
> -    assert (conn->cmds_in_flight != NULL);
> -    cmd = conn->cmds_in_flight;
> -    assert (cmd->handle == be64toh (conn->sbuf.request.handle));
> -    if (cmd->type == NBD_CMD_WRITE) {
> -      conn->wbuf = cmd->data;
> -      conn->wlen = cmd->count;
> -      SET_NEXT_STATE (%SEND_WRITE_PAYLOAD);
> -    }
> -    else
> -      SET_NEXT_STATE (%.READY);
> +  case 0:  SET_NEXT_STATE (%PREPARE_WRITE_PAYLOAD);
>    }
>    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;
> +    SET_NEXT_STATE (%SEND_WRITE_PAYLOAD);
> +  }
> +  else
> +    SET_NEXT_STATE (%FINISH);
> +  return 0;
> +
>   ISSUE_COMMAND.SEND_WRITE_PAYLOAD:
>    switch (send_from_wbuf (conn)) {
>    case -1: SET_NEXT_STATE (%.DEAD); return -1;
> -  case 0:  SET_NEXT_STATE (%.READY);
> +  case 0:  SET_NEXT_STATE (%FINISH);
>    }
>    return 0;
> 
> + ISSUE_COMMAND.FINISH:
> +  struct command_in_flight *cmd;
> +
> +  assert (conn->cmds_to_issue != NULL);
> +  cmd = conn->cmds_to_issue;
> +  conn->cmds_to_issue = cmd->next;
> +  cmd->next = conn->cmds_in_flight;
> +  conn->cmds_in_flight = cmd;
> +  SET_NEXT_STATE (%.READY);
> +  return 0;
> +
>  } /* END STATE MACHINE */

This is a simple state splitting, along with a later move of the
command to the in-flight list.

Since no more commands could be added to the cmds_to_issue queue while
we're not in the READY state, actually I think cmds_to_issue might
just be a single command pointer after all.

The patch however is generally fine.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v