Let's check for a quite a few more errors. Among other things, this adds some coverage for a few things I've patched recently. --- And these enhancements set me up for my next fix: making NBD_CMD_DISC prevent future commands. tests/errors.c | 167 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 154 insertions(+), 13 deletions(-) diff --git a/tests/errors.c b/tests/errors.c index 99d5820..415c378 100644 --- a/tests/errors.c +++ b/tests/errors.c @@ -27,12 +27,40 @@ #include <libnbd.h> +#define MAXSIZE (65 * 1024 * 1024) /* Oversize on purpose */ + +static char *progname; +static char buf[MAXSIZE]; + +static void +check (int experr, const char *prefix) +{ + const char *msg = nbd_get_error (); + int errnum = nbd_get_errno (); + + printf ("error: \"%s\"\n", msg); + printf ("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); + } +} + int main (int argc, char *argv[]) { struct nbd_handle *nbd; - const char *msg; - int errnum; + const char *cmd[] = { "nbdkit", "-s", "--exit-with-parent", "memory", + "size=128m", NULL }; + + progname = argv[0]; nbd = nbd_create (); if (nbd == NULL) { @@ -47,23 +75,136 @@ main (int argc, char *argv[]) argv[0]); exit (EXIT_FAILURE); } - msg = nbd_get_error (); - errnum = nbd_get_errno (); - printf ("error: \"%s\"\n", msg); - printf ("errno: %d (%s)\n", errnum, strerror (errnum)); - if (strncmp (msg, "nbd_pread: ", strlen ("nbd_pread: ")) != 0) { - fprintf (stderr, "%s: test failed: missing context prefix: %s\n", - argv[0], msg); + check (ENOTCONN, "nbd_pread: "); + + /* Request a name that is too long. */ + memset (buf, 'a', 4999); + buf[4999] = '\0'; + if (nbd_set_export_name (nbd, buf) != -1) { + fprintf (stderr, "%s: test failed: " + "nbd_set_export_name did not reject large name\n", + argv[0]); + exit (EXIT_FAILURE); + } + check (ENAMETOOLONG, "nbd_set_export_name: "); + + /* Poll while there is no fd. */ + if (nbd_aio_get_fd (nbd) != -1) { + fprintf (stderr, "%s: test failed: " + "nbd_aio_get_fd did not fail prior to connection\n", + argv[0]); + } + check (EINVAL, "nbd_aio_get_fd: "); + if (nbd_poll (nbd, 1000) != -1) { + fprintf (stderr, "%s: test failed: " + "nbd_poll did not fail prior to connection\n", + argv[0]); + } + check (EINVAL, "nbd_poll: "); + + /* Connect to a working server, then try to connect again. */ + if (nbd_connect_command (nbd, (char **) cmd) == -1) { + fprintf (stderr, "%s: %s\n", argv[0], nbd_get_error ()); + exit (EXIT_FAILURE); + } + if (nbd_connect_command (nbd, (char **) cmd) != -1) { + fprintf (stderr, "%s: test failed: " + "nbd_connect_command did not reject repeat attempt\n", + argv[0]); + exit (EXIT_FAILURE); + } + check (EINVAL, "nbd_connect_command: "); + + /* Try to notify that writes are ready when we aren't blocked on POLLOUT */ + if (nbd_aio_notify_write (nbd) != -1) { + fprintf (stderr, "%s: test failed: " + "nbd_aio_notify_write in wrong state did not fail\n", + argv[0]); + exit (EXIT_FAILURE); + } + check (EINVAL, "nbd_aio_notify_write: "); + + /* Check for status of a bogus handle */ + if (nbd_aio_command_completed (nbd, 0) != -1) { + fprintf (stderr, "%s: test failed: " + "nbd_aio_command_completed on bogus handle did not fail\n", + argv[0]); exit (EXIT_FAILURE); } - if (errnum != ENOTCONN) { + check (EINVAL, "nbd_aio_command_completed: "); + + /* Read from an invalid offset */ + if (nbd_pread (nbd, NULL, 0, -1, 0) != -1) { fprintf (stderr, "%s: test failed: " - "expected errno = ENOTCONN, but got %d\n", - argv[0], errnum); + "nbd_pread did not fail with bogus offset\n", + argv[0]); exit (EXIT_FAILURE); } + check (EINVAL, "nbd_pread: "); - /* XXX Test some more stuff here. */ + /* Use unknown command flags */ + if (nbd_pread (nbd, NULL, 0, 0, -1) != -1) { + fprintf (stderr, "%s: test failed: " + "nbd_pread did not fail with bogus flags\n", + argv[0]); + exit (EXIT_FAILURE); + } + check (EINVAL, "nbd_pread: "); + + /* Check that oversized requests are rejected */ + if (nbd_pread (nbd, buf, MAXSIZE, 0, 0) != -1) { + fprintf (stderr, "%s: test failed: " + "nbd_pread did not fail with oversize request\n", + argv[0]); + exit (EXIT_FAILURE); + } + check (ERANGE, "nbd_pread: "); + if (nbd_aio_pwrite (nbd, buf, MAXSIZE, 0, 0) != -1) { + fprintf (stderr, "%s: test failed: " + "nbd_aio_pwrite did not fail with oversize request\n", + argv[0]); + exit (EXIT_FAILURE); + } + check (ERANGE, "nbd_aio_pwrite: "); + + /* Queue up a write command so large that we block on POLLIN, then queue + * multiple disconnects. XXX The last one should fail. + */ + if (nbd_aio_pwrite (nbd, buf, 2 * 1024 * 1024, 0, 0) == -1) { + fprintf (stderr, "%s: %s\n", argv[0], nbd_get_error ()); + exit (EXIT_FAILURE); + } + if ((nbd_aio_get_direction (nbd) & LIBNBD_AIO_DIRECTION_WRITE) == 0) { + fprintf (stderr, "%s: test failed: " + "expect to be blocked on write\n", + argv[0]); + exit (EXIT_FAILURE); + } + if (nbd_aio_disconnect (nbd, 0) == -1) { + fprintf (stderr, "%s: %s\n", argv[0], nbd_get_error ()); + exit (EXIT_FAILURE); + } + if (nbd_aio_disconnect (nbd, 0) == -1) { /* XXX */ + fprintf (stderr, "%s: %s\n", argv[0], nbd_get_error ()); + exit (EXIT_FAILURE); + } + + /* Flush the queue (whether this one fails is a race with how fast + * the server shuts down, so don't enforce status), then try to send + * another command while CLOSED/DEAD + */ + if (nbd_shutdown (nbd) == -1) { + fprintf (stderr, "%s: ignoring %s\n", argv[0], nbd_get_error ()); + } + else + fprintf (stderr, "%s: shutdown completed successfully\n", argv[0]); + if (nbd_pread (nbd, NULL, 0, 0, 0) != -1) { + fprintf (stderr, "%s: test failed: " + "nbd_pread did not fail on non-connected handle\n", + argv[0]); + exit (EXIT_FAILURE); + } + check (EINVAL, "nbd_pread: "); nbd_close (nbd); exit (EXIT_SUCCESS); -- 2.20.1
Richard W.M. Jones
2019-Jun-28 08:10 UTC
Re: [Libguestfs] [libnbd PATCH] tests: Enhance errors test
ACK Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Richard W.M. Jones
2019-Jun-30 17:54 UTC
Re: [Libguestfs] [libnbd PATCH] tests: Enhance errors test
On Thu, Jun 27, 2019 at 10:18:30PM -0500, Eric Blake wrote:> + /* Queue up a write command so large that we block on POLLIN, then queue > + * multiple disconnects. XXX The last one should fail. > + */ > + if (nbd_aio_pwrite (nbd, buf, 2 * 1024 * 1024, 0, 0) == -1) { > + fprintf (stderr, "%s: %s\n", argv[0], nbd_get_error ()); > + exit (EXIT_FAILURE); > + } > + if ((nbd_aio_get_direction (nbd) & LIBNBD_AIO_DIRECTION_WRITE) == 0) { > + fprintf (stderr, "%s: test failed: " > + "expect to be blocked on write\n", > + argv[0]); > + exit (EXIT_FAILURE); > + }This test fails when run under valgrind. An abbreviated log shows what's happening: libnbd: debug: nbd_aio_pwrite: event CmdIssue: READY -> ISSUE_COMMAND.START libnbd: debug: nbd_aio_pwrite: transition: ISSUE_COMMAND.START -> ISSUE_COMMAND. SEND_REQUEST libnbd: debug: nbd_aio_pwrite: transition: ISSUE_COMMAND.SEND_REQUEST -> ISSUE_C OMMAND.PREPARE_WRITE_PAYLOAD libnbd: debug: nbd_aio_pwrite: transition: ISSUE_COMMAND.PREPARE_WRITE_PAYLOAD -> ISSUE_COMMAND.SEND_WRITE_PAYLOADlibnbd: debug: nbd_aio_pwrite: transition: ISSUE_COMMAND.SEND_WRITE_PAYLOAD -> I SSUE_COMMAND.FINISH libnbd: debug: nbd_aio_pwrite: transition: ISSUE_COMMAND.FINISH -> READY /home/rjones/d/libnbd/tests/.libs/lt-errors: test failed: expect to be blocked on write It seems as if this is caused by valgrinded code running more slowly, rather than an actual valgrind/memory error. I wonder if we could remove the race using a custom nbdkit-sh-plugin which would block on writes until (eg) a local trigger file was touched? Even that seems as if it would depend on the amount of data that the kernel is able to buffer. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Eric Blake
2019-Jul-02 14:01 UTC
Re: [Libguestfs] [libnbd PATCH] tests: Enhance errors test
On 6/30/19 12:54 PM, Richard W.M. Jones wrote:> On Thu, Jun 27, 2019 at 10:18:30PM -0500, Eric Blake wrote: >> + /* Queue up a write command so large that we block on POLLIN, then queue >> + * multiple disconnects. XXX The last one should fail. >> + */ >> + if (nbd_aio_pwrite (nbd, buf, 2 * 1024 * 1024, 0, 0) == -1) { >> + fprintf (stderr, "%s: %s\n", argv[0], nbd_get_error ()); >> + exit (EXIT_FAILURE); >> + } >> + if ((nbd_aio_get_direction (nbd) & LIBNBD_AIO_DIRECTION_WRITE) == 0) { >> + fprintf (stderr, "%s: test failed: " >> + "expect to be blocked on write\n", >> + argv[0]); >> + exit (EXIT_FAILURE); >> + } > > This test fails when run under valgrind. An abbreviated log shows > what's happening: > > libnbd: debug: nbd_aio_pwrite: event CmdIssue: READY -> ISSUE_COMMAND.START > libnbd: debug: nbd_aio_pwrite: transition: ISSUE_COMMAND.START -> ISSUE_COMMAND. > SEND_REQUEST > libnbd: debug: nbd_aio_pwrite: transition: ISSUE_COMMAND.SEND_REQUEST -> ISSUE_C > OMMAND.PREPARE_WRITE_PAYLOAD > libnbd: debug: nbd_aio_pwrite: transition: ISSUE_COMMAND.PREPARE_WRITE_PAYLOAD - >> ISSUE_COMMAND.SEND_WRITE_PAYLOAD > libnbd: debug: nbd_aio_pwrite: transition: ISSUE_COMMAND.SEND_WRITE_PAYLOAD -> I > SSUE_COMMAND.FINISH > libnbd: debug: nbd_aio_pwrite: transition: ISSUE_COMMAND.FINISH -> READY > /home/rjones/d/libnbd/tests/.libs/lt-errors: test failed: expect to be blocked on write > > It seems as if this is caused by valgrinded code running more slowly, > rather than an actual valgrind/memory error.Or even that valgrind's interception of send()/recv() performs buffering differently than we get by default from the kernel. I don't know if running strace on valgrind is a sensible enough thing to do to see syscall behavior?> > I wonder if we could remove the race using a custom nbdkit-sh-plugin > which would block on writes until (eg) a local trigger file was > touched? Even that seems as if it would depend on the amount of data > that the kernel is able to buffer.I don't know how to make an nbdkit plugin stop the code in nbdkit/server from read()ing from the client (the plugin code doesn't get to run until the core has learned that the client wants a command serviced). But it may be possible to tweak things to send back-to-back write requests, where even if the first write request gets sent completely, the plugin can delay responding to that first write and use --filter=noparallel to prevent the second command from reaching nbdkit. I'll play with that, to see if I can reproduce the valgrind race, as well as work around it with back-to-back write commands to increase the likelihood of actually preventing nbdkit from consuming the second command. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Possibly Parallel Threads
- Re: [libnbd PATCH] tests: Enhance errors test
- [libnbd PATCH] tests: Enhance errors test
- [libnbd PATCH 2/3] states: Split ISSUE_COMMAND.SEND_REQUEST
- [libnbd PATCH 3/3] states: Allow in-flight read while writing next command
- [libnbd PATCH v2 4/5] states: Allow in-flight read while writing next command