[Libguestfs] [libnbd PATCH 2/3] api: Add new COVER_OPT_MODE flag to nbd_shutdown

Eric Blake
2023-08-29T22:20:42Z

When nbd_set_opt_mode() was introduced, we did not adjust the behavior
of nbd_shutdown(), so as a result it fails if attempted after the
initial connection but before nbd_opt_abort() or nbd_opt_go().  To
avoid spurious error messages, at least nbdinfo has to perform
special-casing on whether it is in opt mode (call nbd_opt_abort) or
connected (call nbd_shutdown) prior to calling nbd_close.  It would be
nicer to just have a single API perform whatever is needed to
gracefully shut the connection, without having to hard-code the
special casing into each application.

The approach taken here is to add a new flag; if the flag is absent
(behavior of an older application), then nbd_opt_abort must still be
invoked separately before attempting nbd_shutdown.  But if the flag is
present, then whether or not the server supports opt mode, the
shutdown will be graceful.  This patch updates the unit test to show
the effect of the new flag, and the next commit will simplify nbdinfo
by utilizing it.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 generator/API.ml          | 21 ++++++++++++++++-----
 lib/disconnect.c          | 15 +++++++++++++++
 tests/shutdown-opt-mode.c | 27 ++++++++++++++++++++++++++-
 3 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/generator/API.ml b/generator/API.ml
index 1a9c8141..0b837259 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -255,6 +255,7 @@ let shutdown_flags    flag_prefix = "SHUTDOWN";
   flags = [
     "ABANDON_PENDING", 1 lsl 16;
+    "COVER_OPT_MODE", 1 lsl 17;
   ]
 }
 let all_flags = [ cmd_flags; handshake_flags; strict_flags;
@@ -1231,7 +1232,8 @@   "set_opt_mode", {
 starting the connection.  To leave the mode and proceed on to the
 ready state, you must use L<nbd_opt_go(3)> successfully; a failed
 L<nbd_opt_go(3)> returns to the negotiating state to allow a change of
-export name before trying again.  You may also use L<nbd_opt_abort(3)>
+export name before trying again.  You may also use L<nbd_opt_abort(3)>,
+or the C<LIBNBD_SHUTDOWN_COVER_OPT_MODE> flag of
L<nbd_shutdown(3)>,
 to end the connection without finishing negotiation.";
     example = Some "examples/list-exports.c";
     see_also = [Link "get_opt_mode"; Link
"aio_is_negotiating";
@@ -1240,7 +1242,7 @@   "set_opt_mode", {
                 Link "opt_set_meta_context"; Link
"opt_starttls";
                 Link "opt_structured_reply";
                 Link "set_tls"; Link
"set_request_structured_replies";
-                Link "aio_connect"];
+                Link "aio_connect"; Link "shutdown"];
   };

   "get_opt_mode", {
@@ -2667,7 +2669,7 @@   "shutdown", {
     default_call with
     args = []; optargs = [ OFlags ("flags", shutdown_flags, None) ];
     ret = RErr;
-    permitted_states = [ Connected ];
+    permitted_states = [ Negotiating; Connected ];
     modifies_fd = true;
     shortdesc = "disconnect from the NBD server";
     longdesc = "\
@@ -2678,7 +2680,7 @@   "shutdown", {

 This function works whether or not the handle is ready for
 transmission of commands. If more fine-grained control is
-needed, see L<nbd_aio_disconnect(3)>.
+needed, see L<nbd_aio_opt_abort(3)> and L<nbd_aio_disconnect(3)>.

 The C<flags> argument is a bitmask, including zero or more of the
 following shutdown flags:
@@ -2693,12 +2695,21 @@   "shutdown", {
 issuing those commands before informing the server of the intent
 to disconnect.

+=item C<LIBNBD_SHUTDOWN_COVER_OPT_MODE> = 0x20000
+
+If the server is still in option negotiation mode
+(see L<nbd_set_opt_mode(3)>), gracefully abandon the
+connection as if by L<nbd_opt_abort(3)>, instead of
+complaining that the server is not yet fully connected.
+If option negotiation mode was not in use or was
+completed by L<nbd_opt_go(3)>, this flag has no effect.
+
 =back

 For convenience, the constant C<LIBNBD_SHUTDOWN_MASK> is available
 to describe all shutdown flags recognized by this build of libnbd.
 A future version of the library may add new flags.";
-    see_also = [Link "close"; Link "aio_disconnect"];
+    see_also = [Link "close"; Link "aio_disconnect"; Link
"aio_opt_abort"];
     example = Some "examples/reads-and-writes.c";
   };

diff --git a/lib/disconnect.c b/lib/disconnect.c
index 083d6cd3..d250e739 100644
--- a/lib/disconnect.c
+++ b/lib/disconnect.c
@@ -30,6 +30,20 @@
 int
 nbd_unlocked_shutdown (struct nbd_handle *h, uint32_t flags)
 {
+  /* If still negotiating, return an error unless COVER_OPT_ABORT lets
+   * us trigger opt_abort instead.
+   */
+  if (nbd_internal_is_state_negotiating (get_next_state (h))) {
+    if (flags & LIBNBD_SHUTDOWN_COVER_OPT_MODE) {
+      if (nbd_unlocked_aio_opt_abort (h) == -1)
+        return -1;
+      goto wait;
+    }
+    set_error (EINVAL, "invalid state: COVER_OPT_ABORT not requested, but
"
+               "the handle is still negotiating options with the
server");
+    return -1;
+  }
+
   /* If ABANDON_PENDING, abort any commands that have not yet had any
    * bytes sent to the server, so NBD_CMD_DISC becomes next in line.
    */
@@ -50,6 +64,7 @@ nbd_unlocked_shutdown (struct nbd_handle *h, uint32_t flags)
       return -1;
   }

+ wait:
   while (!nbd_internal_is_state_closed (get_next_state (h)) &&
          !nbd_internal_is_state_dead (get_next_state (h))) {
     if (nbd_unlocked_poll (h, -1) == -1)
diff --git a/tests/shutdown-opt-mode.c b/tests/shutdown-opt-mode.c
index 34386220..96547607 100644
--- a/tests/shutdown-opt-mode.c
+++ b/tests/shutdown-opt-mode.c
@@ -119,6 +119,31 @@ main (int argc, char *argv[])
   }
   nbd_close (nbd);

-  /* Part 3: TODO: add flag to nbd_shutdown */
+  /* Part 3: Use of flag avoids the need to manually do opt_abort. */
+  nbd = nbd_create ();
+  if (nbd == NULL) {
+    fprintf (stderr, "%s: %s\n", progname, nbd_get_error ());
+    exit (EXIT_FAILURE);
+  }
+  if (nbd_set_opt_mode (nbd, true) == -1) {
+    fprintf (stderr, "%s: %s\n", progname, nbd_get_error ());
+    exit (EXIT_FAILURE);
+  }
+  if (nbd_connect_command (nbd, (char **)cmd_new) == -1) {
+    fprintf (stderr, "%s: %s\n", progname, nbd_get_error ());
+    exit (EXIT_FAILURE);
+  }
+  if (nbd_aio_is_negotiating (nbd) != 1) {
+    fprintf (stderr, "%s: unexpected state\n", progname);
+    exit (EXIT_FAILURE);
+  }
+
+  /* Shutdown succeeds, because of the flag. */
+  if (nbd_shutdown (nbd, LIBNBD_SHUTDOWN_COVER_OPT_MODE) == -1) {
+    fprintf (stderr, "%s: %s\n", progname, nbd_get_error ());
+    exit (EXIT_FAILURE);
+  }
+  nbd_close (nbd);
+
   exit (EXIT_SUCCESS);
 }
--
2.41.0