On 05/19/2017 01:03 PM, Daniel P. Berrange wrote: > The semantics around handling ipv4=on|off & ipv6=on|off are quite > subtle to understand in combination with the various hostname addresses > and backend types. Introduce a massive test matrix that launches QEMU > and validates the ability to connect a client on each protocol as > appropriate. > > The test requires that the host has ability to bind to both :: and > 0.0.0.0, on port 9000. If either protocol is not available, or if > something is already listening on that port the test will skip. > > Although it isn't using the QTest APIs, it expects the > QTEST_QEMU_BINARY env variable to be set. > > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> > --- > tests/.gitignore | 1 +
Nice - that often gets forgotten. > tests/Makefile.include | 4 + > tests/test-sockets-proto.c | 855 > +++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 860 insertions(+) > create mode 100644 tests/test-sockets-proto.c 'make check' passed for me with your patches (on a system with both IPv4 and IPv6 support), so: Tested-by: Eric Blake <ebl...@redhat.com> I did not try what happens on an IPv4-only system, presumably the test still behaves sanely there (where sanely may mean skipping rather than completing?). > +++ b/tests/test-sockets-proto.c > @@ -0,0 +1,855 @@ > +/* > + * QTest for IPv4/IPv6 protocol setup > + * > + * Copyright (c) 2017 Red Hat, Inc. and/or its affiliates Interesting choice of attribution line. > +/* > + * This is the giant matrix of combinations we need to consider. > + * There are 3 axes we deal with > + * > + * Axis 1: Protocol flags: > + * > + * ipv4=unset, ipv6=unset -> v4 & v6 clients ([1] > + * ipv4=unset, ipv6=off -> v4 clients only > + * ipv4=unset, ipv6=on -> v6 clients only > + * ipv4=off, ipv6=unset -> v6 clients only > + * ipv4=off, ipv6=off -> error - can't disable both [2] > + * ipv4=off, ipv6=on -> v6 clients only > + * ipv4=on, ipv6=unset -> v4 clients only > + * ipv4=on, ipv6=off -> v4 clients only > + * ipv4=on, ipv6=on -> v4 & v6 clients [3] > + * > + * Depending on the listening address, some of those combinations > + * may result in errors. eg ipv4=off,ipv6=on combined with 0.0.0.0 > + * is nonsensical. > + * > + * [1] Some backends only support a single socket listener, so > + * will actually only allow v4 clients > + * [2] QEMU should fail to startup in this case > + * [3] If hostname is "" or "::", then we get a single listener > + * on IPv6 and thus can also accept v4 clients. For all other > + * hostnames, have same problem as [1]. Makes sense. > + * > + * Axis 2: Listening address: > + * > + * "" - resolves to 0.0.0.0 and ::, in that order > + * "0.0.0.0" - v4 clients only > + * "::" - Mostly v6 clients only. Some scenarios should > + * permit v4 clients too. Correct. > + * > + * Axis 3: Backend type: > + * > + * Migration - restricted to a single listener. Also relies > + * on buggy inet_parse() which can't accept > + * =off/=on parameters to ipv4/ipv6 flags > + * Chardevs - restricted to a single listener. > + * VNC - supports multiple listeners. Also supports > + * socket ranges, so has extra set of tests > + * in the matrix And explains the size of the test. Thankfully it doesn't seem to add too much noticeable additional time to 'make check'. > + * > + */ > +static QSocketsData test_data[] = { > + /* Migrate with "" address */ > + /* XXX all settings with =off are disabled due to inet_parse() bug */ > + /* XXX multilistener bug - should be .ipv6 = 1 */ Nice that you've pointed out spots for further improvements, and where we EXPECT the test to change once we improve the code. > +static pid_t run_qemu(const char *args) > +{ > + const char *pidfile = "test-sockets-proto.pid"; > + char *pidstr; > + pid_t child; > + int status; > + pid_t ret; > + const char *binary = getenv("QTEST_QEMU_BINARY"); > + long pid; > + if (binary == NULL) { > + g_printerr("Missing QTEST_QEMU_BINARY env variable"); > + } > + g_assert(binary != NULL); Should we do: if (!binary) { message; exit(1); } instead of relying on g_assert() to do the exit? > + /* Now test IPv6 */ > + child = run_qemu(data->args); > + > + /* > + * The child should always succeed, because its the > + * same config as the succesful run we just did above s/succesful/successful/ > + > +int main(int argc, char **argv) > +{ > + int ret; > + gsize i; > + > + if (check_protocol_support() < 0) { > + return 0; /* Skip test if we can't bind */ We don't have a magic number for skipped tests? Many projects use exit status 77 (rather than 0) to delineate a test that did not fail, but whose skip results cannot be used as conclusive evidence of passing. At any rate, you've answered my question earlier - you do behave sanely if you cannot test both IPv4 and IPv6 on a given host. Typo is worth fixing, but minor enough that I'm comfortable giving: Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature