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