Enhance the regression tests to prove that the completion callback is
not reached if the aio call itself reports an error; only the .free
callback is guaranteed.

Also add some asserts to the library code that may aid future readers
in seeing how we track transfer semantics of a callback.

Goes hand in hand with the documentation cleanups made in the previous
patch, but done separately for ease of backporting as nbd_aio_opt
calls are not in as many releases.

Reported-by: Tage Johansson <tage.j.li...@posteo.net>
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
 lib/opt.c                          |   2 +
 tests/Makefile.am                  |   5 +
 tests/errors-not-negotiating-aio.c | 170 +++++++++++++++++++++++++++++
 .gitignore                         |   1 +
 4 files changed, 178 insertions(+)
 create mode 100644 tests/errors-not-negotiating-aio.c

diff --git a/lib/opt.c b/lib/opt.c
index f58d5e19..1446eef3 100644
--- a/lib/opt.c
+++ b/lib/opt.c
@@ -223,6 +223,7 @@ nbd_unlocked_opt_list (struct nbd_handle *h, 
nbd_list_callback *list)
   if (nbd_unlocked_aio_opt_list (h, &l, &c) == -1)
     return -1;

+  assert (CALLBACK_IS_NULL (l));
   SET_CALLBACK_TO_NULL (*list);
   if (wait_for_option (h) == -1)
     return -1;
@@ -269,6 +270,7 @@ opt_meta_context_queries (struct nbd_handle *h,
   if (aio_opt_meta_context_queries (h, opt, queries, &l, &c) == -1)
     return -1;

+  assert (CALLBACK_IS_NULL (l));
   SET_CALLBACK_TO_NULL (*context);
   if (wait_for_option (h) == -1)
     return -1;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 3a93251e..6eddcb7a 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -171,6 +171,7 @@ check_PROGRAMS += \
        errors-connect-null \
        errors-connect-twice \
        errors-not-negotiating \
+       errors-not-negotiating-aio \
        errors-notify-not-blocked \
        errors-bad-cookie \
        errors-pread-structured \
@@ -243,6 +244,7 @@ TESTS += \
        errors-connect-null \
        errors-connect-twice \
        errors-not-negotiating \
+       errors-not-negotiating-aio \
        errors-notify-not-blocked \
        errors-bad-cookie \
        errors-pread-structured \
@@ -332,6 +334,9 @@ errors_connect_twice_LDADD = $(top_builddir)/lib/libnbd.la
 errors_not_negotiating_SOURCES = errors-not-negotiating.c
 errors_not_negotiating_LDADD = $(top_builddir)/lib/libnbd.la

+errors_not_negotiating_aio_SOURCES = errors-not-negotiating-aio.c
+errors_not_negotiating_aio_LDADD = $(top_builddir)/lib/libnbd.la
+
 errors_notify_not_blocked_SOURCES = errors-notify-not-blocked.c
 errors_notify_not_blocked_LDADD = $(top_builddir)/lib/libnbd.la

diff --git a/tests/errors-not-negotiating-aio.c 
b/tests/errors-not-negotiating-aio.c
new file mode 100644
index 00000000..b09cae82
--- /dev/null
+++ b/tests/errors-not-negotiating-aio.c
@@ -0,0 +1,170 @@
+/* NBD client library in userspace
+ * Copyright Red Hat
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/* Deliberately provoke some errors and check the error messages from
+ * nbd_get_error etc look reasonable.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+
+#include <libnbd.h>
+
+static char *progname;
+
+static void
+check (int experr, const char *prefix)
+{
+  const char *msg = nbd_get_error ();
+  int errnum = nbd_get_errno ();
+
+  fprintf (stderr, "error: \"%s\"\n", msg);
+  fprintf (stderr, "errno: %d (%s)\n", errnum, strerror (errnum));
+  if (strncmp (msg, prefix, strlen (prefix)) != 0) {
+    fprintf (stderr, "%s: test failed: missing context prefix: %s\n",
+             progname, msg);
+    exit (EXIT_FAILURE);
+  }
+  if (errnum != experr) {
+    fprintf (stderr, "%s: test failed: "
+             "expected errno = %d (%s), but got %d\n",
+             progname, experr, strerror (experr), errnum);
+    exit (EXIT_FAILURE);
+  }
+}
+
+struct progress {
+  int id;
+  bool call_freed;
+  bool comp_freed;
+};
+
+static int
+list_call (void *user_data, const char *name, const char *description)
+{
+  struct progress *p = user_data;
+
+  fprintf (stderr, "%s: list callback should not be reached, iter %d\n",
+           progname, p->id);
+  exit (EXIT_FAILURE);
+}
+
+static void
+list_free (void *user_data)
+{
+  struct progress *p = user_data;
+
+  if (p->comp_freed) {
+    fprintf (stderr, "%s: list free called too late, iter %d\n",
+             progname, p->id);
+    exit (EXIT_FAILURE);
+  }
+  p->call_freed = true;
+}
+
+static int
+comp_call (void *user_data, int *error)
+{
+  struct progress *p = user_data;
+
+  fprintf (stderr, "%s: list callback should not be reached, iter %d\n",
+           progname, p->id);
+  exit (EXIT_FAILURE);
+}
+
+static void
+comp_free (void *user_data)
+{
+  struct progress *p = user_data;
+
+  if (!p->call_freed) {
+    fprintf (stderr, "%s: completion free called too early, iter %d\n",
+             progname, p->id);
+    exit (EXIT_FAILURE);
+  }
+  p->comp_freed = true;
+}
+
+int
+main (int argc, char *argv[])
+{
+  struct nbd_handle *nbd;
+  const char *cmd[] = {
+    "nbdkit", "-s", "-v", "--exit-with-parent", "memory", "1048576", NULL
+  };
+  struct progress p;
+  nbd_list_callback list_cb = { .callback = list_call,
+                                .user_data = &p,
+                                .free = list_free };
+  nbd_completion_callback comp_cb = { .callback = comp_call,
+                                      .user_data = &p,
+                                      .free = comp_free };
+
+  progname = argv[0];
+
+  nbd = nbd_create ();
+  if (nbd == NULL) {
+    fprintf (stderr, "%s\n", nbd_get_error ());
+    exit (EXIT_FAILURE);
+  }
+
+  /* Too early to try nbd_aio_opt_*. */
+  p = (struct progress) { .id = 0 };
+  if (nbd_aio_opt_list (nbd, list_cb, comp_cb) != -1) {
+    fprintf (stderr, "%s: test failed: "
+             "nbd_aio_opt_list did not reject attempt in wrong state\n",
+             argv[0]);
+    exit (EXIT_FAILURE);
+  }
+  check (ENOTCONN, "nbd_aio_opt_list: ");
+  if (!p.call_freed || !p.comp_freed) {
+    fprintf (stderr, "%s: test failed: "
+             "nbd_aio_opt_list did not call .free callbacks\n",
+             argv[0]);
+    exit (EXIT_FAILURE);
+  }
+
+  /* Connect to the server without requesting negotiation mode. */
+  if (nbd_connect_command (nbd, (char **)cmd) == -1) {
+    fprintf (stderr, "%s: %s\n", argv[0], nbd_get_error ());
+    exit (EXIT_FAILURE);
+  }
+
+  /* Too late to try nbd_aio_opt_*. */
+  p = (struct progress) { .id = 1 };
+  if (nbd_aio_opt_list (nbd, list_cb, comp_cb) != -1) {
+    fprintf (stderr, "%s: test failed: "
+             "nbd_aio_opt_list did not reject attempt in wrong state\n",
+             argv[0]);
+    exit (EXIT_FAILURE);
+  }
+  check (EINVAL, "nbd_aio_opt_list: ");
+  if (!p.call_freed || !p.comp_freed) {
+    fprintf (stderr, "%s: test failed: "
+             "nbd_aio_opt_list did not call .free callbacks\n",
+             argv[0]);
+    exit (EXIT_FAILURE);
+  }
+
+  nbd_close (nbd);
+  exit (EXIT_SUCCESS);
+}
diff --git a/.gitignore b/.gitignore
index efe3080a..b43e83c0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -220,6 +220,7 @@ Makefile.in
 /tests/errors-name-too-long
 /tests/errors-not-connected
 /tests/errors-not-negotiating
+/tests/errors-not-negotiating-aio
 /tests/errors-notify-not-blocked
 /tests/errors-poll-no-fd
 /tests/errors-pread-structured
-- 
2.41.0

_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Reply via email to