On Mon, Mar 12, 2018 at 12:49:33PM +0000, Daniel P. Berrangé wrote: > From: "Daniel P. Berrange" <berra...@redhat.com> > > The test-io-channel-socket.c file has some useful helper functions for > checking if a specific IP protocol is available. Other tests need to > perform similar kinds of checks to avoid running tests that will fail > due to missing IP protocols. > > Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com> > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> > --- > tests/Makefile.include | 2 +- > tests/socket-helpers.c | 104 > +++++++++++++++++++++++++++++++++++++++++ > tests/socket-helpers.h | 42 +++++++++++++++++ > tests/test-io-channel-socket.c | 72 +--------------------------- > 4 files changed, 149 insertions(+), 71 deletions(-) > create mode 100644 tests/socket-helpers.c > create mode 100644 tests/socket-helpers.h > > diff --git a/tests/Makefile.include b/tests/Makefile.include > index ef9b88c369..3d5b6174a8 100644 > --- a/tests/Makefile.include > +++ b/tests/Makefile.include > @@ -715,7 +715,7 @@ tests/test-crypto-tlssession$(EXESUF): > tests/test-crypto-tlssession.o \ > tests/crypto-tls-x509-helpers.o tests/pkix_asn1_tab.o > $(test-crypto-obj-y) > tests/test-io-task$(EXESUF): tests/test-io-task.o $(test-io-obj-y) > tests/test-io-channel-socket$(EXESUF): tests/test-io-channel-socket.o \ > - tests/io-channel-helpers.o $(test-io-obj-y) > + tests/io-channel-helpers.o tests/socket-helpers.o $(test-io-obj-y) > tests/tpm-crb-test$(EXESUF): tests/tpm-crb-test.o tests/tpm-emu.o > $(test-io-obj-y) > tests/tpm-tis-test$(EXESUF): tests/tpm-tis-test.o tests/tpm-emu.o > $(test-io-obj-y) > tests/test-io-channel-file$(EXESUF): tests/test-io-channel-file.o \ > diff --git a/tests/socket-helpers.c b/tests/socket-helpers.c > new file mode 100644 > index 0000000000..13b6bb38eb > --- /dev/null > +++ b/tests/socket-helpers.c > @@ -0,0 +1,104 @@ > +/* > + * Helper functions for tests using sockets > + * > + * Copyright 2015-2018 Red Hat, Inc. > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation; either version 2 or > + * (at your option) version 3 of the License. > + * > + * This program 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 General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, see <http://www.gnu.org/licenses/>. > + * > + */ > + > +#include "qemu/osdep.h" > +#include "qemu-common.h" > +#include "qemu/sockets.h" > +#include "socket-helpers.h" > + > +#ifndef AI_ADDRCONFIG > +# define AI_ADDRCONFIG 0 > +#endif > +#ifndef EAI_ADDRFAMILY > +# define EAI_ADDRFAMILY 0 > +#endif > + > +int socket_can_bind(const char *hostname) > +{ > + int fd = -1; > + struct addrinfo ai, *res = NULL; > + int rc; > + int ret = -1; > + > + memset(&ai, 0, sizeof(ai)); > + ai.ai_flags = AI_CANONNAME | AI_ADDRCONFIG; > + ai.ai_family = AF_UNSPEC; > + ai.ai_socktype = SOCK_STREAM; > + > + /* lookup */ > + rc = getaddrinfo(hostname, NULL, &ai, &res); > + if (rc != 0) { > + if (rc == EAI_ADDRFAMILY || > + rc == EAI_FAMILY) { > + errno = EADDRNOTAVAIL; > + goto done; > + } > + errno = EINVAL; > + goto cleanup; > + } > + > + fd = qemu_socket(res->ai_family, res->ai_socktype, res->ai_protocol); > + if (fd < 0) { > + goto cleanup; > + } > + > + if (bind(fd, res->ai_addr, res->ai_addrlen) < 0) { > + if (errno == EADDRNOTAVAIL) { > + goto done; > + } > + goto cleanup; > + } > + > + done: > + ret = 0; > + > + cleanup: > + if (fd != -1) { > + close(fd); > + } > + if (res) { > + freeaddrinfo(res); > + } > + return ret; > +} > + > + > +int socket_check_protocol_support(bool *has_ipv4, bool *has_ipv6) > +{ > + *has_ipv4 = *has_ipv6 = false; > + > + if (socket_can_bind("127.0.0.1") < 0) { > + if (errno != EADDRNOTAVAIL) { > + return -1; > + } > + } else { > + *has_ipv4 = true; > + } > + > + if (socket_can_bind("::1") < 0) { > + if (errno != EADDRNOTAVAIL) { > + return -1; > + } > + } else { > + *has_ipv6 = true; > + } > +
Sigh, I should have kept the new code identical to the old code, rather than trying to improve it, as this is in fact broken. The socket_can_bind() is mistakenly returning '0' when EADDRNOTAVAIL is set, so we always set the has_ipv4|6 vars to true. It needs this squashed in: @@ -48,9 +48,9 @@ int socket_can_bind(const char *hostname) if (rc == EAI_ADDRFAMILY || rc == EAI_FAMILY) { errno = EADDRNOTAVAIL; - goto done; + } else { + errno = EINVAL; } - errno = EINVAL; goto cleanup; } @@ -60,13 +60,9 @@ int socket_can_bind(const char *hostname) } if (bind(fd, res->ai_addr, res->ai_addrlen) < 0) { - if (errno == EADDRNOTAVAIL) { - goto done; - } goto cleanup; } - done: ret = 0; cleanup: Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|