Richard W.M. Jones
2021-Jun-30 19:37 UTC
[Libguestfs] [PATCH libnbd] lib/uri.c: nbd_get_uri: Do not translate port name into service
Because I forgot to use the NI_NUMERICSERV flag, getnameinfo would
translate the port number into a service name (when possible) using
/etc/services. This is not really desirable since raw port numbers
are more portable and descriptive. This also caused occasional test
failures when pick-a-port happened to pick a port number which
coincided with a service name.
This commit also adds a test. Previous to the fix, the test failed with:
$ ./run tests/aio-connect-port
tests/.libs/aio-connect-port: actual URI nbd://127.0.0.1:sieve-filter/ !=
expected URI nbd://127.0.0.1:2000/
This test cannot be run as part of the test suite because it requires
a fixed port number.
See also
https://listman.redhat.com/archives/libguestfs/2021-June/msg00205.html
Reported-by: Martin Kletzander
---
.gitignore | 1 +
lib/uri.c | 3 +-
tests/Makefile.am | 9 +++
tests/aio-connect-port.c | 128 +++++++++++++++++++++++++++++++++++++++
4 files changed, 140 insertions(+), 1 deletion(-)
diff --git a/.gitignore b/.gitignore
index 865b689..602438d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -155,6 +155,7 @@ Makefile.in
/stamp-h1
/test-driver
/tests/aio-connect
+/tests/aio-connect-port
/tests/aio-parallel
/tests/aio-parallel-load
/tests/aio-parallel-load-tls
diff --git a/lib/uri.c b/lib/uri.c
index 9b97e46..bcecbad 100644
--- a/lib/uri.c
+++ b/lib/uri.c
@@ -422,7 +422,8 @@ nbd_unlocked_get_uri (struct nbd_handle *h)
uri.scheme = using_tls ? "nbds" : "nbd";
err = getnameinfo ((struct sockaddr *) &h->connaddr,
h->connaddrlen,
- host, sizeof host, serv, sizeof serv, NI_NUMERICHOST);
+ host, sizeof host, serv, sizeof serv,
+ NI_NUMERICHOST | NI_NUMERICSERV);
if (err != 0) {
set_error (0, "getnameinfo: %s", gai_strerror (err));
goto out;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 09de079..aad5513 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -203,6 +203,7 @@ check_PROGRAMS += \
connect-tcp \
connect-tcp6 \
aio-connect \
+ aio-connect-port \
aio-parallel \
aio-parallel-load \
synch-parallel \
@@ -252,6 +253,9 @@ TESTS += \
closure-lifetimes \
$(NULL)
+# This test is compiled but not run because it requires a fixed port:
+# aio-connect-port
+
errors_SOURCES = errors.c
errors_CPPFLAGS = -I$(top_srcdir)/include
errors_CFLAGS = $(WARNINGS_CFLAGS)
@@ -489,6 +493,11 @@ aio_connect_CPPFLAGS = -I$(top_srcdir)/include
aio_connect_CFLAGS = $(WARNINGS_CFLAGS)
aio_connect_LDADD = $(top_builddir)/lib/libnbd.la
+aio_connect_port_SOURCES = aio-connect-port.c
+aio_connect_port_CPPFLAGS = -I$(top_srcdir)/include
+aio_connect_port_CFLAGS = $(WARNINGS_CFLAGS)
+aio_connect_port_LDADD = $(top_builddir)/lib/libnbd.la
+
aio_parallel_SOURCES = aio-parallel.c
aio_parallel_CPPFLAGS = \
-I$(top_srcdir)/include \
diff --git a/tests/aio-connect-port.c b/tests/aio-connect-port.c
new file mode 100644
index 0000000..5f9613e
--- /dev/null
+++ b/tests/aio-connect-port.c
@@ -0,0 +1,128 @@
+/* NBD client library in userspace
+ * Copyright (C) 2013-2021 Red Hat Inc.
+ *
+ * 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
+ */
+
+/* Test connecting to a fixed IPv4 TCP port (2000) using
+ * nbd_aio_connect. This test is compiled but not normally run
+ * because of the requirement for the fixed port number.
+ *
+ * See also:
+ * https://listman.redhat.com/archives/libguestfs/2021-June/msg00205.html
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+
+#include <libnbd.h>
+
+#define PIDFILE "aio-connect-port.pid"
+
+int
+main (int argc, char *argv[])
+{
+ struct nbd_handle *nbd;
+ int port = 2000;
+ const char *port_str = "2000";
+ pid_t pid;
+ size_t i;
+ struct sockaddr_in addr;
+ char *actual_uri, *expected_uri;
+
+ unlink (PIDFILE);
+
+ pid = fork ();
+ if (pid == -1) {
+ perror ("fork");
+ exit (EXIT_FAILURE);
+ }
+ if (pid == 0) {
+ execlp ("nbdkit",
+ "nbdkit", "-f", "-p", port_str,
"-P", PIDFILE,
+ "--exit-with-parent", "null", NULL);
+ perror ("nbdkit");
+ _exit (EXIT_FAILURE);
+ }
+
+ /* Wait for nbdkit to start listening. */
+ for (i = 0; i < 60; ++i) {
+ if (access (PIDFILE, F_OK) == 0)
+ break;
+ sleep (1);
+ }
+ unlink (PIDFILE);
+
+ nbd = nbd_create ();
+ if (nbd == NULL) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (nbd_supports_uri (nbd) != 1) {
+ fprintf (stderr, "skip: compiled without URI support\n");
+ exit (77);
+ }
+
+ addr.sin_family = AF_INET;
+ addr.sin_addr.s_addr = htonl (INADDR_LOOPBACK);
+ addr.sin_port = htons (port);
+
+ if (nbd_aio_connect (nbd, (struct sockaddr *) &addr, sizeof addr) == -1)
{
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+
+ /* Wait until we have connected. */
+ while (!nbd_aio_is_ready (nbd)) {
+ if (nbd_poll (nbd, -1) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ }
+
+ /* libnbd should be able to construct a URI for this connection. */
+ if (asprintf (&expected_uri, "nbd://127.0.0.1:%s/", port_str)
== -1) {
+ perror ("asprintf");
+ exit (EXIT_FAILURE);
+ }
+ actual_uri = nbd_get_uri (nbd);
+ if (actual_uri == NULL) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (strcmp (actual_uri, expected_uri) != 0) {
+ fprintf (stderr, "%s: actual URI %s != expected URI %s\n",
+ argv[0], actual_uri, expected_uri);
+ exit (EXIT_FAILURE);
+ }
+ free (actual_uri);
+ free (expected_uri);
+
+ if (nbd_shutdown (nbd, 0) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+
+ nbd_close (nbd);
+ exit (EXIT_SUCCESS);
+}
--
2.32.0
Eric Blake
2021-Jun-30 21:41 UTC
[Libguestfs] [PATCH libnbd] lib/uri.c: nbd_get_uri: Do not translate port name into service
On Wed, Jun 30, 2021 at 08:37:46PM +0100, Richard W.M. Jones wrote:> Because I forgot to use the NI_NUMERICSERV flag, getnameinfo would > translate the port number into a service name (when possible) using > /etc/services. This is not really desirable since raw port numbers > are more portable and descriptive. This also caused occasional test > failures when pick-a-port happened to pick a port number which > coincided with a service name. > > This commit also adds a test. Previous to the fix, the test failed with: > > $ ./run tests/aio-connect-port > tests/.libs/aio-connect-port: actual URI nbd://127.0.0.1:sieve-filter/ != expected URI nbd://127.0.0.1:2000/ > > This test cannot be run as part of the test suite because it requires > a fixed port number. > > See also > https://listman.redhat.com/archives/libguestfs/2021-June/msg00205.html > > Reported-by: Martin Kletzander > --- > .gitignore | 1 + > lib/uri.c | 3 +- > tests/Makefile.am | 9 +++ > tests/aio-connect-port.c | 128 +++++++++++++++++++++++++++++++++++++++ > 4 files changed, 140 insertions(+), 1 deletion(-)ACK. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org