On 03/24/22 10:13, Richard W.M. Jones wrote: This patch removes qemu-nbd support, as the first step. "test-virt-p2v.sh" is simply removed, as the remaining "test-virt-p2v-nbdkit.sh" is mostly identical. Ref: https://listman.redhat.com/archives/libguestfs/2022-March/028475.html Suggested-by: Richard W.M. Jones <rjones(a)redhat.com&gt; Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- Makefile.am | 2 - contrib/aux-scripts/do-build.sh | 7 +- contrib/patches/0002-RHEL-5-ONLY-QEMU-NBD-1.4-HAS-NO-f-OPTION.patch | 34 ----- dependencies.m4 | 6 +- docs/p2v-building.pod | 9 +- main.c | 2 +- nbd.c | 136 +------------------- test-virt-p2v-nbdkit.sh | 5 +- test-virt-p2v-scp.sh | 2 +- test-virt-p2v-ssh.sh | 4 +- test-virt-p2v.sh | 56 -------- virt-p2v.pod | 29 ++--- 12 files changed, 31 insertions(+), 261 deletions(-) diff --git a/Makefile.am b/Makefile.am index 2080890b14e3..e98129e99028 100644 --- a/Makefile.am +++ b/Makefile.am @@ -27,36 +27,35 @@ SUBDIRS += bash EXTRA_DIST = \ $(BUILT_SOURCES) \ $(TESTS) $(LIBGUESTFS_TESTS) $(SLOW_TESTS) \ AUTHORS \ contrib/aux-scripts/do-build.sh \ contrib/build-p2v-iso.sh \ contrib/patches/0001-RHEL-5-ONLY-DISABLE-AUTOMATIC-REMOTE-PORT-ALLOCATION.patch \ - contrib/patches/0002-RHEL-5-ONLY-QEMU-NBD-1.4-HAS-NO-f-OPTION.patch \ contrib/test-p2v-iso.sh \ dependencies.m4 \ generate-p2v-authors.pl \ generate-p2v-config.pl \ issue \ kiwi-config.sh \ kiwi-config.xml.in \ launch-virt-p2v \ libguestfs/README \ miniexpect/README \ p2v.ks.in \ p2v.service \ podcheck.pl \ test-functions.sh \ test-virt-p2v-docs.sh \ test-virt-p2v-pxe.sshd_config.in \ test-virt-p2v-scp.sh \ test-virt-p2v-ssh.sh \ virt-p2v.pod \ virt-p2v-make-disk.in \ virt-p2v-make-disk.pod \ virt-p2v-make-kickstart.in \ virt-p2v-make-kickstart.pod \ virt-p2v-make-kiwi.in \ virt-p2v-make-kiwi.pod # Don't clean ssh_host_rsa_key{,.pub} or id_rsa{,.pub} since those # consume system entropy to regenerate. @@ -339,7 +338,6 @@ TESTS = \ test-virt-p2v-docs.sh LIBGUESTFS_TESTS = \ - test-virt-p2v.sh \ test-virt-p2v-nbdkit.sh if HAVE_LIBGUESTFS diff --git a/contrib/aux-scripts/do-build.sh b/contrib/aux-scripts/do-build.sh index 53a844d9c99d..0ef2ab185beb 100644 --- a/contrib/aux-scripts/do-build.sh +++ b/contrib/aux-scripts/do-build.sh @@ -49,22 +49,21 @@ pushd libguestfs-* # Various hacks for different versions of RHEL. case $osversion in rhel-5.*|centos-5.*) # This just forces configure to ignore these missing dependencies. export LIBTINFO_CFLAGS=-D_GNU_SOURCE export LIBTINFO_LIBS=-lncurses export JANSSON_CFLAGS=-D_GNU_SOURCE export JANSSON_LIBS=-ljansson # Remove some unsupported flags that the configure script hard codes. sed -i -e 's/-fno-strict-overflow//' configure sed -i -e 's/-Wno-strict-overflow//' configure # Apply some RHEL 5 only patches. patch -p1 < ../patches/0001-RHEL-5-ONLY-DISABLE-AUTOMATIC-REMOTE-PORT-ALLOCATION.patch - patch -p1 < ../patches/0002-RHEL-5-ONLY-QEMU-NBD-1.4-HAS-NO-f-OPTION.patch ;; rhel-6.*|centos-6.*) # This just forces configure to ignore these missing dependencies. export LIBTINFO_CFLAGS=-D_GNU_SOURCE export LIBTINFO_LIBS=-lncurses export JANSSON_CFLAGS=-D_GNU_SOURCE export JANSSON_LIBS=-ljansson ;; @@ -106,41 +105,43 @@ popd # More hacks for different versions of RHEL. case $osversion in rhel-5.*|centos-5.*) # RHEL 5 livecd-tools is broken with syslinux, this fixes it: sed -i -e 's,/usr/lib/syslinux/,/usr/share/syslinux/,g'\ /usr/lib/python2.4/site-packages/imgcreate/live.py # livecd-tools cannot parse certain aspects of the kickstart: sed -i \ -e 's/--plaintext//g' \ -e 's/^firewall.*//g' \ -e 's/^%end.*//g' \ p2v.ks # Remove some packages which don't exist on RHEL 5: sed -i \ -e 's,^dracut-live.*,,g' \ -e 's,^dejavu-.*,,g' \ -e 's,^mesa-dri-drivers.*,,g' \ -e 's,^network-manager-applet.*,,g' \ -e 's,^nm-connection-editor.*,,g' \ - -e 's,^/usr/bin/qemu-nbd.*,,g' \ + -e 's,^/usr/sbin/nbdkit.*,,g' \ + -e 's,^/usr/lib64/nbdkit/plugins/nbdkit-file-plugin.so.*,,g' \ -e '/^net-tools/a syslinux' \ p2v.ks # Remove systemctl lines, doesn't exist on RHEL 5. sed -i \ -e 's/^\(systemctl.*\)/#\1/g' \ p2v.ks ;; rhel-6.*|centos-6.*) # Remove some packages which don't exist on RHEL 6: sed -i \ -e 's,^dracut-live.*,,g' \ -e 's,^firewalld.*,,g' \ -e 's,^network-manager-applet.*,,g' \ -e 's,^nm-connection-editor.*,,g' \ - -e 's,^/usr/bin/qemu-nbd.*,,g' \ + -e 's,^/usr/sbin/nbdkit.*,,g' \ + -e 's,^/usr/lib64/nbdkit/plugins/nbdkit-file-plugin.so.*,,g' \ p2v.ks # Remove systemctl lines, doesn't exist on RHEL 5. sed -i \ -e 's/^\(systemctl.*\)/#\1/g' \ p2v.ks ;; diff --git a/contrib/patches/0002-RHEL-5-ONLY-QEMU-NBD-1.4-HAS-NO-f-OPTION.patch b/contrib/patches/0002-RHEL-5-ONLY-QEMU-NBD-1.4-HAS-NO-f-OPTION.patch deleted file mode 100644 index d0bc2cfc9ec2..000000000000 --- a/contrib/patches/0002-RHEL-5-ONLY-QEMU-NBD-1.4-HAS-NO-f-OPTION.patch +++ /dev/null @@ -1,34 +0,0 @@ -From 3ccd25c981431426038d7952f5b0b86118d92c23 Mon Sep 17 00:00:00 2001 -From: "Richard W.M. Jones" <rjones(a)redhat.com&gt; -Date: Sat, 21 Jan 2017 05:57:17 -0500 -Subject: [PATCH 2/2] RHEL 5 ONLY QEMU-NBD 1.4 HAS NO -f OPTION - ---- - p2v/nbd.c | 4 ++-- - 1 file changed, 2 insertions(+), 2 deletions(-) - -diff --git a/p2v/nbd.c b/p2v/nbd.c -index b1caf2f..2232b08 100644 ---- a/p2v/nbd.c -+++ b/p2v/nbd.c -@@ -409,7 +409,7 @@ start_qemu_nbd (const char *device, - "-r", /* readonly (vital!) */ - "-p", port_str, /* listening port */ - "-t", /* persistent */ -- "-f", "raw", /* force raw format */ -+ //"-f", "raw", /* force raw format */ - "-b", ipaddr, /* listen only on loopback interface */ - "--cache=unsafe", /* use unsafe caching for speed */ - device, /* a device like /dev/sda */ -@@ -424,7 +424,7 @@ start_qemu_nbd (const char *device, - "qemu-nbd", - "-r", /* readonly (vital!) */ - "-t", /* persistent */ -- "-f", "raw", /* force raw format */ -+ //"-f", "raw", /* force raw format */ - "--cache=unsafe", /* use unsafe caching for speed */ - device, /* a device like /dev/sda */ - NULL); --- -1.8.2.3 - diff --git a/dependencies.m4 b/dependencies.m4 index 40b3786cb0b7..20d068028dbe 100644 --- a/dependencies.m4 +++ b/dependencies.m4 @@ -23,39 +23,40 @@ dnl only runs on the virt-v2v conversion server. ifelse(REDHAT,1, dnl Used by the virt-p2v binary. pcre libxml2 gtk`'GTK_VERSION dbus-libs dnl Run as external programs by the p2v binary. /usr/bin/ssh - /usr/bin/qemu-nbd + /usr/sbin/nbdkit + /usr/lib64/nbdkit/plugins/nbdkit-file-plugin.so which dnl Generally useful tools to use within xterm vim-minimal dnl Useful disk and diagnostic utilities. iscsi-initiator-utils dnl X11 environment /usr/bin/xinit /usr/bin/Xorg xorg-x11-drivers xorg-x11-fonts-Type1 dejavu-sans-fonts dejavu-sans-mono-fonts mesa-dri-drivers metacity NetworkManager nm-connection-editor network-manager-applet dnl dbus is required by nm-applet, but not a dependency in Fedora dbus-x11 dnl sysadmins prefer ifconfig net-tools dnl RHBZ#1157679 @hardware-support ) @@ -128,33 +129,34 @@ ifelse(SUSE,1, ifelse(OPENMANDRIVA,1, dnl Used by the virt-p2v binary. pcre libxml2 gtk`'GTK_VERSION dbus-libs dnl Run as external programs by the p2v binary. /usr/bin/ssh - /usr/bin/qemu-nbd + /usr/sbin/nbdkit + /usr/lib64/nbdkit/plugins/nbdkit-file-plugin.so which dnl Generally useful tools to use within xterm vim-enhanced dnl X11 environment /usr/bin/xinit /usr/bin/Xorg xorg-x11-drivers xorg-x11-fonts-Type1 dejavu-sans-fonts dejavu-sans-mono-fonts mesa-dri-drivers kwin_x11 NetworkManager nm-connection-editor network-manager-applet dnl dbus is required by nm-applet, but not a dependency in Fedora dbus-x11 dnl sysadmins prefer ifconfig net-tools ) diff --git a/docs/p2v-building.pod b/docs/p2v-building.pod index f249f742554a..21edc0255015 100644 --- a/docs/p2v-building.pod +++ b/docs/p2v-building.pod @@ -73,15 +73,12 @@ I<Required>. I<Required>. -=item qemu-nbd - =item nbdkit -Optional. qemu-nbd is used for testing. +Optional. -L<virt-p2v(1)> requires either qemu-nbd or nbdkit, but these only need -to be present on the virt-p2v ISO, they do not need to be installed at -compile time. +L<virt-p2v(1)> requires nbdkit, but it only needs to be present on the +virt-p2v ISO, it does not need to be installed at compile time. =item Gtk E<ge> 2.24, or 3 diff --git a/main.c b/main.c index a28a8b64ddaa..e4e9c1fe21c1 100644 --- a/main.c +++ b/main.c @@ -87,27 +87,27 @@ static void __attribute__((noreturn)) usage (int status) { if (status != EXIT_SUCCESS) fprintf (stderr, _("Try ‘%s --help’ for more information.\n"), g_get_prgname ()); else { printf (_("%s: Convert a physical machine to use KVM\n" "Copyright (C) 2009-2019 Red Hat Inc.\n" "Usage:\n" " %s [--options]\n" "Options:\n" " --help Display brief help\n" " --cmdline=CMDLINE Used to debug command line parsing\n" " --colors|--colours Use ANSI colour sequences even if not tty\n" " --iso Running in the ISO environment\n" - " --nbd=qemu-nbd,nbdkit Search order for NBD servers\n" + " --nbd=nbdkit Search order for NBD servers\n" " --test-disk=DISK.IMG For testing, use disk as /dev/sda\n" " -v|--verbose Verbose messages\n" " -V|--version Display version and exit\n" "For more information, see the manpage %s(1).\n"), g_get_prgname (), g_get_prgname (), g_get_prgname ()); } exit (status); } /* XXX Copied from fish/options.c. */ diff --git a/nbd.c b/nbd.c index db6228a8aba0..46f6304d2c51 100644 --- a/nbd.c +++ b/nbd.c @@ -1,47 +1,47 @@ /* virt-p2v * Copyright (C) 2009-2019 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 of the License, or * (at your option) any later version. * * 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 <https://www.gnu.org/licenses/>;. */ /** * This file handles the virt-p2v I<--nbd> command line option - * and running either L<qemu-nbd(8)> or L<nbdkit(1)>. + * and running L<nbdkit(1)>. */ #include <config.h> #include <stdio.h> #include <stdlib.h> #include <string.h> #include <inttypes.h> #include <fcntl.h> #include <unistd.h> #include <time.h> #include <netdb.h> #include <errno.h> #include <error.h> #include <libintl.h> #include <sys/types.h> #include <sys/socket.h> #include <assert.h> #include "p2v.h" /* How long to wait for the NBD server to start (seconds). */ #define WAIT_NBD_TIMEOUT 10 /* The local port that the NBD server listens on (incremented for * each server which is started). */ @@ -50,10 +50,8 @@ static int nbd_local_port; /* List of servers specified by the --nbd option. */ enum nbd_server { /* 0 is reserved for "end of list" */ - QEMU_NBD = 1, - QEMU_NBD_NO_SA = 2, - NBDKIT = 3, - NBDKIT_NO_SA = 4, + NBDKIT = 1, + NBDKIT_NO_SA = 2, }; static enum nbd_server *cmdline_servers = NULL; @@ -61,32 +59,29 @@ static const char * nbd_server_string (enum nbd_server s) { const char *ret = NULL; switch (s) { - case QEMU_NBD: ret = "qemu-nbd"; break; - case QEMU_NBD_NO_SA: ret = "qemu-nbd-no-sa"; break; case NBDKIT: ret = "nbdkit"; break; case NBDKIT_NO_SA: ret = "nbdkit-no-sa"; break; } if (ret == NULL) abort (); return ret; } /* If no --nbd option is passed, we use this standard list instead. * Must match the documentation in virt-p2v(1). */ static const enum nbd_server standard_servers[] = - { QEMU_NBD, QEMU_NBD_NO_SA, NBDKIT, NBDKIT_NO_SA, 0 }; + { NBDKIT, NBDKIT_NO_SA, 0 }; /* After testing the list of servers passed by the user, this is * server we decide to use. */ static enum nbd_server use_server; -static pid_t start_qemu_nbd (const char *device, const char *ipaddr, int port, int *fds, size_t nr_fds); static pid_t start_nbdkit (const char *device, const char *ipaddr, int port, int *fds, size_t nr_fds); static int get_local_port (void); static int open_listening_socket (const char *ipaddr, int **fds, size_t *nr_fds); @@ -131,44 +126,40 @@ void set_nbd_option (const char *opt) { size_t i, len; CLEANUP_FREE_STRING_LIST char **strs = NULL; if (cmdline_servers != NULL) error (EXIT_FAILURE, 0, _("--nbd option appears multiple times")); strs = guestfs_int_split_string (',', opt); if (strs == NULL) error (EXIT_FAILURE, errno, _("malloc")); len = guestfs_int_count_strings (strs); if (len == 0) error (EXIT_FAILURE, 0, _("--nbd option cannot be empty")); cmdline_servers = malloc (sizeof (enum nbd_server) * (len + 1)); if (cmdline_servers == NULL) error (EXIT_FAILURE, errno, _("malloc")); for (i = 0; strs[i] != NULL; ++i) { - if (STREQ (strs[i], "qemu-nbd") || STREQ (strs[i], "qemu")) - cmdline_servers[i] = QEMU_NBD; - else if (STREQ (strs[i], "qemu-nbd-no-sa") || STREQ (strs[i], "qemu-no-sa")) - cmdline_servers[i] = QEMU_NBD_NO_SA; - else if (STREQ (strs[i], "nbdkit")) + if (STREQ (strs[i], "nbdkit")) cmdline_servers[i] = NBDKIT; else if (STREQ (strs[i], "nbdkit-no-sa")) cmdline_servers[i] = NBDKIT_NO_SA; else error (EXIT_FAILURE, 0, _("--nbd: unknown server: %s"), strs[i]); } assert (i == len); cmdline_servers[i] = 0; /* marks the end of the list */ } /** * Test the I<--nbd> option (or built-in default list) to see which * servers are actually installed and appear to be working. * * Set the C<use_server> global accordingly. */ @@ -176,113 +167,88 @@ void test_nbd_servers (void) { size_t i; int r; const enum nbd_server *servers; /* Initialize nbd_local_port. */ if (is_iso_environment) /* The p2v ISO should allow us to open up just about any port, so * we can fix a port number in that case. Using a predictable * port number in this case should avoid rare errors if the port * colides with another (ie. it'll either always fail or never * fail). */ nbd_local_port = 50123; else /* When testing on the local machine, choose a random port. */ nbd_local_port = 50000 + (random () % 10000); if (cmdline_servers != NULL) servers = cmdline_servers; else servers = standard_servers; use_server = 0; for (i = 0; servers[i] != 0; ++i) { #if DEBUG_STDERR fprintf (stderr, "checking for %s ...\n", nbd_server_string (servers[i])); #endif switch (servers[i]) { - case QEMU_NBD: /* with socket activation */ - r = system ("qemu-nbd --version" -#ifndef DEBUG_STDERR - " >/dev/null 2>&1" -#endif - " && grep -sq LISTEN_PID `which qemu-nbd`" - ); - if (r == 0) { - use_server = servers[i]; - goto finish; - } - break; - - case QEMU_NBD_NO_SA: - r = system ("qemu-nbd --version" -#ifndef DEBUG_STDERR - " >/dev/null 2>&1" -#endif - ); - if (r == 0) { - use_server = servers[i]; - goto finish; - } - break; - case NBDKIT: /* with socket activation */ r = system ("nbdkit file --version" #ifndef DEBUG_STDERR " >/dev/null 2>&1" #endif " && grep -sq LISTEN_PID `which nbdkit`" ); if (r == 0) { use_server = servers[i]; goto finish; } break; case NBDKIT_NO_SA: r = system ("nbdkit file --version" #ifndef DEBUG_STDERR " >/dev/null 2>&1" #endif ); if (r == 0) { use_server = servers[i]; goto finish; } break; default: abort (); } } finish: if (use_server == 0) { fprintf (stderr, _("%s: no working NBD server was found, cannot continue.\n" "Please check the --nbd option in the virt-p2v(1) man page.\n"), g_get_prgname ()); exit (EXIT_FAILURE); } /* Release memory used by the --nbd option. */ free (cmdline_servers); cmdline_servers = NULL; #if DEBUG_STDERR fprintf (stderr, "picked %s\n", nbd_server_string (use_server)); #endif } /** * Start the NBD server. * * We previously tested all NBD servers (see C<test_nbd_servers>) and * hopefully found one which will work. * * Returns the process ID (E<gt> 0) or C<0> if there is an error. */ @@ -290,56 +256,35 @@ pid_t start_nbd_server (const char **ipaddr, int *port, const char *device) { int *fds = NULL; size_t i, nr_fds; pid_t pid; switch (use_server) { - case QEMU_NBD: /* qemu-nbd with socket activation */ - /* Ideally we would bind this socket to "localhost", but that - * requires two listening FDs, and qemu-nbd currently cannot - * support socket activation with two FDs. So we only bind to the - * IPv4 address. - */ - *ipaddr = "127.0.0.1"; - *port = open_listening_socket (*ipaddr, &fds, &nr_fds); - if (*port == -1) return -1; - pid = start_qemu_nbd (device, *ipaddr, *port, fds, nr_fds); - for (i = 0; i < nr_fds; ++i) - close (fds[i]); - free (fds); - return pid; - - case QEMU_NBD_NO_SA: /* qemu-nbd without socket activation */ - *ipaddr = "localhost"; - *port = get_local_port (); - if (*port == -1) return -1; - return start_qemu_nbd (device, *ipaddr, *port, NULL, 0); - case NBDKIT: /* nbdkit with socket activation */ *ipaddr = "localhost"; *port = open_listening_socket (*ipaddr, &fds, &nr_fds); if (*port == -1) return -1; pid = start_nbdkit (device, *ipaddr, *port, fds, nr_fds); for (i = 0; i < nr_fds; ++i) close (fds[i]); free (fds); return pid; case NBDKIT_NO_SA: /* nbdkit without socket activation */ *ipaddr = "localhost"; *port = get_local_port (); if (*port == -1) return -1; return start_nbdkit (device, *ipaddr, *port, NULL, 0); } abort (); } #define FIRST_SOCKET_ACTIVATION_FD 3 /** * Set up file descriptors and environment variables for * socket activation. * * Note this function runs in the child between fork and exec. */ @@ -347,103 +292,32 @@ static inline void socket_activation (int *fds, size_t nr_fds) { size_t i; char nr_fds_str[16]; char pid_str[16]; if (fds == NULL) return; for (i = 0; i < nr_fds; ++i) { int fd = FIRST_SOCKET_ACTIVATION_FD + i; if (fds[i] != fd) { dup2 (fds[i], fd); close (fds[i]); } } snprintf (nr_fds_str, sizeof nr_fds_str, "%zu", nr_fds); setenv ("LISTEN_FDS", nr_fds_str, 1); snprintf (pid_str, sizeof pid_str, "%d", (int) getpid ()); setenv ("LISTEN_PID", pid_str, 1); } -/** - * Start a local L<qemu-nbd(1)> process. - * - * If we are using socket activation, C<fds> and C<nr_fds> will - * contain the locally pre-opened file descriptors for this. - * Otherwise if C<fds == NULL> we pass the port number. - * - * Returns the process ID (E<gt> 0) or C<0> if there is an error. - */ -static pid_t -start_qemu_nbd (const char *device, - const char *ipaddr, int port, int *fds, size_t nr_fds) -{ - pid_t pid; - char port_str[64]; - -#if DEBUG_STDERR - fprintf (stderr, "starting qemu-nbd for %s on %s:%d%s\n", - device, ipaddr, port, - fds == NULL ? "" : " using socket activation"); -#endif - - snprintf (port_str, sizeof port_str, "%d", port); - - pid = fork (); - if (pid == -1) { - set_nbd_error ("fork: %m"); - return 0; - } - - if (pid == 0) { /* Child. */ - close (0); - if (open ("/dev/null", O_RDONLY) == -1) { - perror ("open: /dev/null"); - _exit (EXIT_FAILURE); - } - - if (fds == NULL) { /* without socket activation */ - execlp ("qemu-nbd", - "qemu-nbd", - "-r", /* readonly (vital!) */ - "-p", port_str, /* listening port */ - "-t", /* persistent */ - "-f", "raw", /* force raw format */ - "-b", ipaddr, /* listen only on loopback interface */ - "--cache=unsafe", /* use unsafe caching for speed */ - device, /* a device like /dev/sda */ - NULL); - perror ("qemu-nbd"); - _exit (EXIT_FAILURE); - } - else { /* socket activation */ - socket_activation (fds, nr_fds); - - execlp ("qemu-nbd", - "qemu-nbd", - "-r", /* readonly (vital!) */ - "-t", /* persistent */ - "-f", "raw", /* force raw format */ - "--cache=unsafe", /* use unsafe caching for speed */ - device, /* a device like /dev/sda */ - NULL); - perror ("qemu-nbd"); - _exit (EXIT_FAILURE); - } - } - - /* Parent. */ - return pid; -} - /** * Start a local L<nbdkit(1)> process using the * L<nbdkit-file-plugin(1)>. * * If we are using socket activation, C<fds> and C<nr_fds> will * contain the locally pre-opened file descriptors for this. * Otherwise if C<fds == NULL> we pass the port number. * * Returns the process ID (E<gt> 0) or C<0> if there is an error. */ diff --git a/test-virt-p2v-nbdkit.sh b/test-virt-p2v-nbdkit.sh index 9adb195f9249..8e91d45c4014 100755 --- a/test-virt-p2v-nbdkit.sh +++ b/test-virt-p2v-nbdkit.sh @@ -1,28 +1,27 @@ #!/bin/bash - # libguestfs virt-p2v test script # Copyright (C) 2014-2019 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 of the License, or # (at your option) any later version. # # 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 <https://www.gnu.org/licenses/>;. -# Test virt-p2v in non-GUI mode using nbdkit instead of qemu-nbd. +# Test virt-p2v in non-GUI mode using nbdkit. set -e $TEST_FUNCTIONS skip_if_skipped skip_if_backend uml -skip_unless nbdkit file --version skip_unless test -f fedora.img skip_unless test -f blank-part.img @@ -47,7 +46,7 @@ export PATH=$d:$PATH # The Linux kernel command line. cmdline="p2v.server=localhost p2v.name=fedora p2v.disks=$f1,$f2 p2v.o=local p2v.os=$(pwd)/$d p2v.network=em1:wired,other p2v.post=" -# Only use nbdkit, disable qemu-nbd. +# Only use nbdkit. $VG virt-p2v --cmdline="$cmdline" --nbd=nbdkit,nbdkit-no-sa # Test the libvirt XML metadata and a disk was created. diff --git a/test-virt-p2v-scp.sh b/test-virt-p2v-scp.sh index ca851012fb85..65ed018cdfb8 100755 --- a/test-virt-p2v-scp.sh +++ b/test-virt-p2v-scp.sh @@ -1,20 +1,20 @@ #!/bin/bash - # Copyright (C) 2014-2019 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 of the License, or # (at your option) any later version. # # 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 <https://www.gnu.org/licenses/>;. -# This is an scp substitute used by test-virt-p2v.sh. +# This is an scp substitute used by test-virt-p2v-nbdkit.sh. TEMP=`getopt \ -o 'o:P:' \ diff --git a/test-virt-p2v-ssh.sh b/test-virt-p2v-ssh.sh index 2442b7d3f2c6..8a14b71fbd4b 100755 --- a/test-virt-p2v-ssh.sh +++ b/test-virt-p2v-ssh.sh @@ -1,20 +1,20 @@ #!/bin/bash - # Copyright (C) 2014 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 of the License, or # (at your option) any later version. # # 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 <https://www.gnu.org/licenses/>;. -# This is an ssh substitute used by test-virt-p2v.sh. +# This is an ssh substitute used by test-virt-p2v-nbdkit.sh. TEMP=`getopt \ -o 'l:No:p:R:' \ @@ -28,29 +28,29 @@ eval set -- "$TEMP" while true ; do case "$1" in # Regular arguments that we can just ignore. -N) shift ;; -l|-o|-p) shift 2 ;; # ssh -R 0:localhost:<port> (port forwarding). Don't actually # port forward, just return the original port number here so that - # the conversion process connects directly to qemu-nbd. + # the conversion process connects directly to nbdkit. -R) arg="$2" port="$(echo $arg | awk -F: '{print $3}')" echo "Allocated port" $port "for remote forward" shift 2 ;; --) shift break ;; *) echo "$0: internal error ($1)" exit 1 ;; esac diff --git a/test-virt-p2v.sh b/test-virt-p2v.sh deleted file mode 100755 index 9629e1b0235a..000000000000 --- a/test-virt-p2v.sh +++ /dev/null @@ -1,56 +0,0 @@ -#!/bin/bash - -# libguestfs virt-p2v test script -# Copyright (C) 2014 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 of the License, or -# (at your option) any later version. -# -# 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 <https://www.gnu.org/licenses/>;. - -# Test virt-p2v in non-GUI mode. - -set -e - -$TEST_FUNCTIONS -skip_if_skipped -skip_if_backend uml -skip_unless test -f fedora.img -skip_unless test -f blank-part.img - -f1="$abs_builddir/fedora.img" -f2="$abs_builddir/blank-part.img" - -d=test-virt-p2v.d -rm -rf $d -mkdir $d - -# We don't want the program under test to run real 'ssh' or 'scp'. -# They won't work. Therefore create dummy 'ssh' and 'scp' binaries. -pushd $d -ln -sf "$abs_srcdir/test-virt-p2v-ssh.sh" ssh -ln -sf "$abs_srcdir/test-virt-p2v-scp.sh" scp -popd -export PATH=$d:$PATH - -# Note that the PATH already contains the local virt-p2v & virt-v2v -# binaries under test (because of the ./run script). - -# The Linux kernel command line. -cmdline="p2v.server=localhost p2v.name=fedora p2v.disks=$f1,$f2 p2v.o=local p2v.os=$(pwd)/$d p2v.network=em1:wired,other p2v.post=" - -$VG virt-p2v --cmdline="$cmdline" - -# Test the libvirt XML metadata and a disk was created. -test -f $d/fedora.xml -test -f $d/fedora-sda -test -f $d/fedora-sdb - -rm -r $d diff --git a/virt-p2v.pod b/virt-p2v.pod index 5f4aa5763524..eb220cbf1cee 100644 --- a/virt-p2v.pod +++ b/virt-p2v.pod @@ -509,19 +509,10 @@ features such as the Shutdown popup button. =item B<--nbd=server[,server...]> Select which NBD server is used. By default the following servers are -checked and the first one found is used: -I<--nbd=qemu-nbd,qemu-nbd-no-sa,nbdkit,nbdkit-no-sa> +checked and the first one found is used: I<--nbd=nbdkit,nbdkit-no-sa> =over 4 -=item B<qemu-nbd> - -Use qemu-nbd. - -=item B<qemu-nbd-no-sa> - -Use qemu-nbd, but disable socket activation. - =item B<nbdkit> Use nbdkit with the file plugin (see: L<nbdkit-file-plugin(1)>). @@ -532,9 +523,9 @@ Use nbdkit, but disable socket activation =back -The C<*-no-sa> variants allow virt-p2v to fall back to older versions -of qemu-nbd and nbdkit which did not support -L<socket activation|http://0pointer.de/blog/projects/socket-activation.html>. +The C<nbdkit-no-sa> variant allows virt-p2v to fall back to older +versions of nbdkit which did not support L<socket +activation|http://0pointer.de/blog/projects/socket-activation.html>. =item B<--test-disk=/PATH/TO/DISK.IMG> @@ -679,22 +670,21 @@ Before conversion actually begins, virt-p2v then makes one or more further ssh connections to the server for data transfer. The transfer protocol used currently is NBD (Network Block Device), -which is proxied over ssh. The NBD server is L<qemu-nbd(1)> by -default but others can be selected using the I<--nbd> command line -option. +which is proxied over ssh. The NBD server is L<nbdkit(1)>; socket +activation can be controlled using the I<--nbd> command line option. There is one ssh connection per physical hard disk on the source machine (the common case — a single hard disk — is shown below): ┌──────────────┐ ┌─────────────────┐ │ virt-p2v │ │ virt-v2v │ │ (physical │ control connection │ (conversion │ │ server) ╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍▶ server) │ │ │ │ │ │ │ data connection │ │ │ ╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍▶ │ - │qemu-nbd ← ─┘ │ │└─ ← NBD │ - │/dev/sda │ │ requests │ + │ nbdkit ← ─┘ │ │└─ ← NBD │ + │ /dev/sda │ │ requests │ ∼ ∼ ∼ ∼ └──────────────┘ └─────────────────┘ @@ -708,7 +698,7 @@ that virt-v2v via libguestfs can open nbd connections which directly read the hard disk(s) of the physical server. Two layers of protection are used to ensure that there are no writes -to the hard disks: Firstly, the qemu-nbd I<-r> (readonly) option is +to the hard disks: Firstly, the nbdkit I<-r> (readonly) option is used. Secondly libguestfs creates an overlay on top of the NBD connection which stores writes in a temporary file on the conversion file. @@ -731,7 +721,6 @@ L<virt-p2v-make-disk(1)>, L<virt-p2v-make-kickstart(1)>, L<virt-p2v-make-kiwi(1)>, L<virt-v2v(1)>, -L<qemu-nbd(1)>, L<nbdkit(1)>, L<nbdkit-file-plugin(1)>, L<ssh(1)>, L<sshd(8)>, -- 2.19.1.3.g30247aa5d201

On 03/31/22 14:27, Richard W.M. Jones wrote: I used pathnames rather than package names just to stick with the tradition of "/usr/bin/qemu-nbd". Secondly, there is no package called "nbdkit-file-plugin"; it's called "nbdkit-basic-plugins". Do you mean that "nbdkit-file-plugin" is resolvable due to the work done for <https://bugzilla.redhat.com/show_bug.cgi?id=2059291> (and before that, manually listed Provides:?) ... I think that dnf behaves very misleadingly here. Consider: (Because I *did* try that yesterday.) At the same time: This is very non-intuitive. A virtual "provides" apparently satisfies "install", but does not satisfy "search". IMO that's simply a bug in dnf, one that keeps tricking me. Anyway, I'll update the deps. Back to your email: On 03/31/22 14:27, Richard W.M. Jones wrote: OK. I removed this line because the qemu-nbd based "sibling" test case, "test-virt-p2v.sh", which this patch removes, has a hard dependency on qemu-nbd (i.e., it does not include a similar "skip_unless" line), despite the docs listing qemu-nbd as optional too. And now with this patch, nbdkit takes the place of qemu-nbd. If your argument is that the lack of "skip_unless qemu-nbd --version" is a preexistent bug in "test-virt-p2v.sh", then I agree to keep "skip_unless nbdkit file --version" here. Thanks! Laszlo

According to the first commit message in this series, remove support for parsing "--nbd=nbdkit-no-sa". Note that there is a non-intuitive change in this patch: in the test_nbd_servers() function, we remove the grepping for the "LISTEN_PID" string in the "nbdkit" binary. In other words, we stop verifying whether "nbdkit" indeed supports socket activation, we just assume it. The primary reason for this is that this check never actually worked when the nbdkit binary was exposed from the root directory of an nbdkit build tree! That binary is actually built from "wrapper.c", and it's only the "server/nbdkit" binary that matches "LISTEN_PID". However, as "wrapper.c" explains, "server/nbdkit" would use the system-wide nbdkit plugins, not the just-built ones, therefore only the wrapper "nbdkit" executable can be used -- and so the "LISTEN_PID" grep command has always been unable to deal with that situation. Unless we remove that check too, with the rest of this patch, virt-p2v refuses to start up -- as no usable nbd server is found. This patch permits further simplification, which will be done subsequently in this series. Ref: https://listman.redhat.com/archives/libguestfs/2022-March/028475.html Suggested-by: Richard W.M. Jones <rjones(a)redhat.com&gt; Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- nbd.c | 42 +------------------- test-virt-p2v-nbdkit.sh | 2 +- virt-p2v.pod | 18 +++------ 3 files changed, 8 insertions(+), 54 deletions(-) diff --git a/nbd.c b/nbd.c index 46f6304d2c51..edc3ab51d889 100644 --- a/nbd.c +++ b/nbd.c @@ -50,8 +50,7 @@ static int nbd_local_port; /* List of servers specified by the --nbd option. */ enum nbd_server { /* 0 is reserved for "end of list" */ NBDKIT = 1, - NBDKIT_NO_SA = 2, }; static enum nbd_server *cmdline_servers = NULL; @@ -59,31 +58,29 @@ static const char * nbd_server_string (enum nbd_server s) { const char *ret = NULL; switch (s) { case NBDKIT: ret = "nbdkit"; break; - case NBDKIT_NO_SA: ret = "nbdkit-no-sa"; break; } if (ret == NULL) abort (); return ret; } /* If no --nbd option is passed, we use this standard list instead. * Must match the documentation in virt-p2v(1). */ static const enum nbd_server standard_servers[] = - { NBDKIT, NBDKIT_NO_SA, 0 }; + { NBDKIT, 0 }; /* After testing the list of servers passed by the user, this is * server we decide to use. */ static enum nbd_server use_server; static pid_t start_nbdkit (const char *device, const char *ipaddr, int port, int *fds, size_t nr_fds); -static int get_local_port (void); static int open_listening_socket (const char *ipaddr, int **fds, size_t *nr_fds); static int bind_tcpip_socket (const char *ipaddr, const char *port, int **fds, size_t *nr_fds); static int connect_with_source_port (const char *hostname, int dest_port, int source_port); @@ -126,40 +123,38 @@ void set_nbd_option (const char *opt) { size_t i, len; CLEANUP_FREE_STRING_LIST char **strs = NULL; if (cmdline_servers != NULL) error (EXIT_FAILURE, 0, _("--nbd option appears multiple times")); strs = guestfs_int_split_string (',', opt); if (strs == NULL) error (EXIT_FAILURE, errno, _("malloc")); len = guestfs_int_count_strings (strs); if (len == 0) error (EXIT_FAILURE, 0, _("--nbd option cannot be empty")); cmdline_servers = malloc (sizeof (enum nbd_server) * (len + 1)); if (cmdline_servers == NULL) error (EXIT_FAILURE, errno, _("malloc")); for (i = 0; strs[i] != NULL; ++i) { if (STREQ (strs[i], "nbdkit")) cmdline_servers[i] = NBDKIT; - else if (STREQ (strs[i], "nbdkit-no-sa")) - cmdline_servers[i] = NBDKIT_NO_SA; else error (EXIT_FAILURE, 0, _("--nbd: unknown server: %s"), strs[i]); } assert (i == len); cmdline_servers[i] = 0; /* marks the end of the list */ } /** * Test the I<--nbd> option (or built-in default list) to see which * servers are actually installed and appear to be working. * * Set the C<use_server> global accordingly. */ @@ -167,88 +162,75 @@ void test_nbd_servers (void) { size_t i; int r; const enum nbd_server *servers; /* Initialize nbd_local_port. */ if (is_iso_environment) /* The p2v ISO should allow us to open up just about any port, so * we can fix a port number in that case. Using a predictable * port number in this case should avoid rare errors if the port * colides with another (ie. it'll either always fail or never * fail). */ nbd_local_port = 50123; else /* When testing on the local machine, choose a random port. */ nbd_local_port = 50000 + (random () % 10000); if (cmdline_servers != NULL) servers = cmdline_servers; else servers = standard_servers; use_server = 0; for (i = 0; servers[i] != 0; ++i) { #if DEBUG_STDERR fprintf (stderr, "checking for %s ...\n", nbd_server_string (servers[i])); #endif switch (servers[i]) { case NBDKIT: /* with socket activation */ r = system ("nbdkit file --version" #ifndef DEBUG_STDERR " >/dev/null 2>&1" -#endif - " && grep -sq LISTEN_PID `which nbdkit`" - ); - if (r == 0) { - use_server = servers[i]; - goto finish; - } - break; - - case NBDKIT_NO_SA: - r = system ("nbdkit file --version" -#ifndef DEBUG_STDERR - " >/dev/null 2>&1" #endif ); if (r == 0) { use_server = servers[i]; goto finish; } break; default: abort (); } } finish: if (use_server == 0) { fprintf (stderr, _("%s: no working NBD server was found, cannot continue.\n" "Please check the --nbd option in the virt-p2v(1) man page.\n"), g_get_prgname ()); exit (EXIT_FAILURE); } /* Release memory used by the --nbd option. */ free (cmdline_servers); cmdline_servers = NULL; #if DEBUG_STDERR fprintf (stderr, "picked %s\n", nbd_server_string (use_server)); #endif } /** * Start the NBD server. * * We previously tested all NBD servers (see C<test_nbd_servers>) and * hopefully found one which will work. * * Returns the process ID (E<gt> 0) or C<0> if there is an error. */ @@ -256,35 +238,29 @@ pid_t start_nbd_server (const char **ipaddr, int *port, const char *device) { int *fds = NULL; size_t i, nr_fds; pid_t pid; switch (use_server) { case NBDKIT: /* nbdkit with socket activation */ *ipaddr = "localhost"; *port = open_listening_socket (*ipaddr, &fds, &nr_fds); if (*port == -1) return -1; pid = start_nbdkit (device, *ipaddr, *port, fds, nr_fds); for (i = 0; i < nr_fds; ++i) close (fds[i]); free (fds); return pid; - - case NBDKIT_NO_SA: /* nbdkit without socket activation */ - *ipaddr = "localhost"; - *port = get_local_port (); - if (*port == -1) return -1; - return start_nbdkit (device, *ipaddr, *port, NULL, 0); } abort (); } #define FIRST_SOCKET_ACTIVATION_FD 3 /** * Set up file descriptors and environment variables for * socket activation. * * Note this function runs in the child between fork and exec. */ @@ -325,89 +301,73 @@ static pid_t start_nbdkit (const char *device, const char *ipaddr, int port, int *fds, size_t nr_fds) { pid_t pid; char port_str[64]; CLEANUP_FREE char *file_str = NULL; #if DEBUG_STDERR fprintf (stderr, "starting nbdkit for %s on %s:%d%s\n", device, ipaddr, port, fds == NULL ? "" : " using socket activation"); #endif snprintf (port_str, sizeof port_str, "%d", port); if (asprintf (&file_str, "file=%s", device) == -1) error (EXIT_FAILURE, errno, "asprintf"); pid = fork (); if (pid == -1) { set_nbd_error ("fork: %m"); return 0; } if (pid == 0) { /* Child. */ close (0); if (open ("/dev/null", O_RDONLY) == -1) { perror ("open: /dev/null"); _exit (EXIT_FAILURE); } if (fds == NULL) { /* without socket activation */ execlp ("nbdkit", "nbdkit", "-r", /* readonly (vital!) */ "-p", port_str, /* listening port */ "-i", ipaddr, /* listen only on loopback interface */ "-f", /* don't fork */ "file", /* file plugin */ file_str, /* a device like file=/dev/sda */ NULL); perror ("nbdkit"); _exit (EXIT_FAILURE); } else { /* socket activation */ socket_activation (fds, nr_fds); execlp ("nbdkit", "nbdkit", "-r", /* readonly (vital!) */ "-f", /* don't fork */ "file", /* file plugin */ file_str, /* a device like file=/dev/sda */ NULL); perror ("nbdkit"); _exit (EXIT_FAILURE); } } /* Parent. */ return pid; } -/** - * This is used when we are starting an NBD server that does not - * support socket activation. We have to pass the '-p' option to - * the NBD server, but there's no good way to choose a free port, - * so we have to just guess. - * - * Returns the port number on success or C<-1> on error. - */ -static int -get_local_port (void) -{ - int port = nbd_local_port; - nbd_local_port++; - return port; -} - /** * This is used when we are starting an NBD server which supports * socket activation. We can open a listening socket on an unused * local port and return it. * * Returns the port number on success or C<-1> on error. * * The file descriptor(s) bound are returned in the array *fds, *nr_fds. * The caller must free the array. */ diff --git a/test-virt-p2v-nbdkit.sh b/test-virt-p2v-nbdkit.sh index 8e91d45c4014..99bb8d9fe293 100755 --- a/test-virt-p2v-nbdkit.sh +++ b/test-virt-p2v-nbdkit.sh @@ -47,7 +47,7 @@ export PATH=$d:$PATH cmdline="p2v.server=localhost p2v.name=fedora p2v.disks=$f1,$f2 p2v.o=local p2v.os=$(pwd)/$d p2v.network=em1:wired,other p2v.post=" # Only use nbdkit. -$VG virt-p2v --cmdline="$cmdline" --nbd=nbdkit,nbdkit-no-sa +$VG virt-p2v --cmdline="$cmdline" --nbd=nbdkit # Test the libvirt XML metadata and a disk was created. test -f $d/fedora.xml diff --git a/virt-p2v.pod b/virt-p2v.pod index eb220cbf1cee..e6cbb1514edc 100644 --- a/virt-p2v.pod +++ b/virt-p2v.pod @@ -509,24 +509,17 @@ features such as the Shutdown popup button. =item B<--nbd=server[,server...]> Select which NBD server is used. By default the following servers are -checked and the first one found is used: I<--nbd=nbdkit,nbdkit-no-sa> +checked and the first one found is used: I<--nbd=nbdkit>. =over 4 =item B<nbdkit> -Use nbdkit with the file plugin (see: L<nbdkit-file-plugin(1)>). - -=item B<nbdkit-no-sa> - -Use nbdkit, but disable socket activation +Use nbdkit with the file plugin (see: L<nbdkit-file-plugin(1)>) and +socket activation. =back -The C<nbdkit-no-sa> variant allows virt-p2v to fall back to older -versions of nbdkit which did not support L<socket -activation|http://0pointer.de/blog/projects/socket-activation.html>. - =item B<--test-disk=/PATH/TO/DISK.IMG> For testing or debugging purposes, replace F</dev/sda> with a local @@ -670,8 +663,9 @@ Before conversion actually begins, virt-p2v then makes one or more further ssh connections to the server for data transfer. The transfer protocol used currently is NBD (Network Block Device), -which is proxied over ssh. The NBD server is L<nbdkit(1)>; socket -activation can be controlled using the I<--nbd> command line option. +which is proxied over ssh. The NBD server is L<nbdkit(1)>, with +L<nbdkit-file-plugin(1)> and L<socket +activation|http://0pointer.de/blog/projects/socket-activation.html>. There is one ssh connection per physical hard disk on the source machine (the common case — a single hard disk — is shown below): -- 2.19.1.3.g30247aa5d201

On 03/31/22 14:52, Richard W.M. Jones wrote: Will do. Thanks! Laszlo

On Thu, Mar 31, 2022 at 09:22:04AM +0200, Laszlo Ersek wrote: Reviewed-by: Richard W.M. Jones <rjones(a)redhat.com&gt; 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

Commit d43f482adce2 ("p2v: avoid connecting to ourself while probing qemu-nbd (RHBZ#1167774)", 2014-12-15) resolved the following race condition: start_conversion() [conversion.c] open_data_connection() [ssh.c] *local_port = nbd_local_port; nbd_local_port++; start_qemu_nbd() [conversion.c] fork() /* in the child */ /* execute "qemu-nbd ... -p port_str" */ bind() wait_qemu_nbd() [conversion.c] connect() The race was between the child process (qemu-nbd) reaching bind() and the parent process reaching connect(). In case the parent process "won", then the kernel would assign an ephemeral source port to the connecting socket from such a pool of ports from which the child process had not bound its listening socket yet. Arguably due to a kernel bug, the kernel could then assign the same port number to the local end of the connecting socket as the port number the socket was connecting to, effectively causing the parent process to connect to itself. This race no longer exists: due to the previous patches, only socket activation is possible, in which bind() is called by the parent process for the listening socket strictly before the parent process calls connect(): start_conversion() [conversion.c] start_nbd_server() [nbd.c] open_listening_socket() [nbd.c] bind_tcpip_socket() [nbd.c] bind() start_nbdkit() [nbd.c] fork() /* in the child */ socket_activation() /* execute "nbdkit", letting it * inherit the already bound socket */ wait_for_nbd_server_to_start() [nbd.c] connect_with_source_port() [nbd.c] bind_source_port() [nbd.c] bind() connect() Remove bind_source_port(). Rename connect_with_source_port() to connect_to_nbdkit(), and remove its "source_port" parameter. Ref: https://listman.redhat.com/archives/libguestfs/2022-March/028475.html Suggested-by: Richard W.M. Jones <rjones(a)redhat.com&gt; Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- nbd.c | 74 +------------------- 1 file changed, 3 insertions(+), 71 deletions(-) diff --git a/nbd.c b/nbd.c index cb849e75dc84..f64afdc53dd3 100644 --- a/nbd.c +++ b/nbd.c @@ -83,8 +83,7 @@ static enum nbd_server use_server; static pid_t start_nbdkit (const char *device, int *fds, size_t nr_fds); static int open_listening_socket (int **fds, size_t *nr_fds); static int bind_tcpip_socket (const char *port, int **fds, size_t *nr_fds); -static int connect_with_source_port (int dest_port, int source_port); -static int bind_source_port (int sockfd, int family, int source_port); +static int connect_to_nbdkit (int dest_port); static char *nbd_error; @@ -467,178 +466,111 @@ int wait_for_nbd_server_to_start (int port) { int sockfd = -1; int result = -1; time_t start_t, now_t; struct timespec half_sec = { .tv_sec = 0, .tv_nsec = 500000000 }; struct timeval timeout = { .tv_usec = 0 }; char magic[8]; /* NBDMAGIC */ size_t bytes_read = 0; ssize_t recvd; time (&start_t); for (;;) { time (&now_t); if (now_t - start_t >= WAIT_NBD_TIMEOUT) { set_nbd_error ("timed out waiting for NBD server to start"); goto cleanup; } - /* Source port for probing NBD server should be one greater than - * port. It's not guaranteed to always bind to this port, but it - * will hint the kernel to start there and try incrementally - * higher ports if needed. This avoids the case where the kernel - * selects port as our source port, and we immediately connect to - * ourself. See: - * https://bugzilla.redhat.com/show_bug.cgi?id=1167774#c9 - */ - sockfd = connect_with_source_port (port, port+1); + sockfd = connect_to_nbdkit (port); if (sockfd >= 0) break; nanosleep (&half_sec, NULL); } time (&now_t); timeout.tv_sec = (start_t + WAIT_NBD_TIMEOUT) - now_t; if (setsockopt (sockfd, SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof timeout) == -1) { set_nbd_error ("waiting for NBD server to start: " "setsockopt(SO_RCVTIMEO): %m"); goto cleanup; } do { recvd = recv (sockfd, magic, sizeof magic - bytes_read, 0); if (recvd == -1) { set_nbd_error ("waiting for NBD server to start: recv: %m"); goto cleanup; } bytes_read += recvd; } while (bytes_read < sizeof magic); if (memcmp (magic, "NBDMAGIC", sizeof magic) != 0) { set_nbd_error ("waiting for NBD server to start: " "'NBDMAGIC' was not received from NBD server"); goto cleanup; } result = 0; cleanup: if (sockfd >= 0) close (sockfd); return result; } /** * Connect to C<localhost:dest_port>, resolving the address using * L<getaddrinfo(3)>. * - * This also sets the source port of the connection to the first free - * port number E<ge> C<source_port>. - * * This may involve multiple connections - to IPv4 and IPv6 for * instance. */ static int -connect_with_source_port (int dest_port, int source_port) +connect_to_nbdkit (int dest_port) { struct addrinfo hints; struct addrinfo *results, *rp; char dest_port_str[16]; int r, sockfd = -1; - int reuseaddr = 1; snprintf (dest_port_str, sizeof dest_port_str, "%d", dest_port); memset (&hints, 0, sizeof hints); hints.ai_family = AF_UNSPEC; /* allow IPv4 or IPv6 */ hints.ai_socktype = SOCK_STREAM; hints.ai_flags = AI_NUMERICSERV; /* numeric dest port number */ hints.ai_protocol = 0; /* any protocol */ r = getaddrinfo ("localhost", dest_port_str, &hints, &results); if (r != 0) { set_nbd_error ("getaddrinfo: localhost/%s: %s", dest_port_str, gai_strerror (r)); return -1; } for (rp = results; rp != NULL; rp = rp->ai_next) { sockfd = socket (rp->ai_family, rp->ai_socktype, rp->ai_protocol); if (sockfd == -1) continue; - /* If we run p2v repeatedly (say, running the tests in a loop), - * there's a decent chance we'll end up trying to bind() to a port - * that is in TIME_WAIT from a prior run. Handle that gracefully - * with SO_REUSEADDR. - */ - if (setsockopt (sockfd, SOL_SOCKET, SO_REUSEADDR, - &reuseaddr, sizeof reuseaddr) == -1) - perror ("warning: setsockopt"); - - /* Need to bind the source port. */ - if (bind_source_port (sockfd, rp->ai_family, source_port) == -1) { - close (sockfd); - sockfd = -1; - continue; - } - /* Connect. */ if (connect (sockfd, rp->ai_addr, rp->ai_addrlen) == -1) { set_nbd_error ("waiting for NBD server to start: " "connect to localhost/%s: %m", dest_port_str); close (sockfd); sockfd = -1; continue; } break; } freeaddrinfo (results); return sockfd; } - -static int -bind_source_port (int sockfd, int family, int source_port) -{ - struct addrinfo hints; - struct addrinfo *results, *rp; - char source_port_str[16]; - int r; - - snprintf (source_port_str, sizeof source_port_str, "%d", source_port); - - memset (&hints, 0, sizeof (hints)); - hints.ai_family = family; - hints.ai_socktype = SOCK_STREAM; - hints.ai_flags = AI_PASSIVE | AI_NUMERICSERV; /* numeric port number */ - hints.ai_protocol = 0; /* any protocol */ - - r = getaddrinfo ("localhost", source_port_str, &hints, &results); - if (r != 0) { - set_nbd_error ("getaddrinfo (bind): localhost/%s: %s", - source_port_str, gai_strerror (r)); - return -1; - } - - for (rp = results; rp != NULL; rp = rp->ai_next) { - if (bind (sockfd, rp->ai_addr, rp->ai_addrlen) == 0) - goto bound; - } - - set_nbd_error ("waiting for NBD server to start: " - "bind to source port %d: %m", - source_port); - freeaddrinfo (results); - return -1; - - bound: - freeaddrinfo (results); - return 0; -} -- 2.19.1.3.g30247aa5d201

Replace nbd_server_string() with its only possible output "nbdkit". Ref: https://listman.redhat.com/archives/libguestfs/2022-March/028475.html Suggested-by: Richard W.M. Jones <rjones(a)redhat.com&gt; Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- nbd.c | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/nbd.c b/nbd.c index 7d8abf3cbbb0..48926d4aec1d 100644 --- a/nbd.c +++ b/nbd.c @@ -50,21 +50,6 @@ enum nbd_server { NBDKIT = 1, }; -static const char * -nbd_server_string (enum nbd_server s) -{ - const char *ret = NULL; - - switch (s) { - case NBDKIT: ret = "nbdkit"; break; - } - - if (ret == NULL) - abort (); - - return ret; -} - /* We use this standard list of nbd server types. Must match the documentation * in virt-p2v(1). */ @@ -121,67 +106,67 @@ void test_nbd_servers (void) { size_t i; int r; const enum nbd_server *servers; /* Initialize nbd_local_port. */ if (is_iso_environment) /* The p2v ISO should allow us to open up just about any port, so * we can fix a port number in that case. Using a predictable * port number in this case should avoid rare errors if the port * colides with another (ie. it'll either always fail or never * fail). */ nbd_local_port = 50123; else /* When testing on the local machine, choose a random port. */ nbd_local_port = 50000 + (random () % 10000); servers = standard_servers; use_server = 0; for (i = 0; servers[i] != 0; ++i) { #if DEBUG_STDERR - fprintf (stderr, "checking for %s ...\n", nbd_server_string (servers[i])); + fprintf (stderr, "checking for nbdkit ...\n"); #endif switch (servers[i]) { case NBDKIT: /* with socket activation */ r = system ("nbdkit file --version" #ifndef DEBUG_STDERR " >/dev/null 2>&1" #endif ); if (r == 0) { use_server = servers[i]; goto finish; } break; default: abort (); } } finish: if (use_server == 0) { fprintf (stderr, _("%s: no working NBD server was found, cannot continue.\n"), g_get_prgname ()); exit (EXIT_FAILURE); } #if DEBUG_STDERR - fprintf (stderr, "picked %s\n", nbd_server_string (use_server)); + fprintf (stderr, "picked nbdkit\n"); #endif } /** * Start the NBD server. * * We previously tested all NBD servers (see C<test_nbd_servers>) and * hopefully found one which will work. * * Returns the process ID (E<gt> 0) or C<0> if there is an error. */ -- 2.19.1.3.g30247aa5d201

We currently track the PIDs of the NBD servers (nbdkit only, at this point) so that we can kill and reap them in cleanup_data_conns(). cleanup_data_conns() is called from three kinds of contexts: (1) in start_conversion(), followed immediately by exit (EXIT_FAILURE); (2) at the end of start_conversion() in GUI mode, where we return to start_conversion_thread(), and then the conversion thread -- a detached thread -- exits almost immediately; (3) at the end of start_conversion() in kernel mode. Here we return to kernel_conversion(), then main(), and soon exit the process. As shown above, whenever we intend to kill and reap the nbdkit processes, in any of these contexts, the parent *thread's* disappearance is also imminent. This gives rise to the idea of (a) removing the tracking & explicit killing of nbdkit processes by PID, and (b) telling the kernel to kill the nbdkit processes *upon* parent thread disappearance. And that's what the "--exit-with-parent" option of nbdkit does -- at least on Linux <https://libguestfs.org/nbdkit-captive.1.html#EXIT-WITH-PARENT> --, so replace the tracking with "--exit-with-parent". ... I have no clue how to test this patch (or how the patch affects non-Linux host OSes). Ref: https://listman.redhat.com/archives/libguestfs/2022-March/028475.html Suggested-by: Richard W.M. Jones <rjones(a)redhat.com&gt; Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- conversion.c | 12 +++--------- nbd.c | 9 +++++---- p2v.h | 1 - 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/conversion.c b/conversion.c index 8bc4119fb47c..da162d8690e3 100644 --- a/conversion.c +++ b/conversion.c @@ -155,267 +155,267 @@ int start_conversion (struct config *config, void (*notify_ui) (int type, const char *data)) { int ret = -1; int status; size_t i, len; const size_t nr_disks = guestfs_int_count_strings (config->disks); time_t now; struct tm tm; CLEANUP_FREE struct data_conn *data_conns = NULL; CLEANUP_FREE char *remote_dir = NULL; char tmpdir[] = "/tmp/p2v.XXXXXX"; char name_file[] = "/tmp/p2v.XXXXXX/name"; char physical_xml_file[] = "/tmp/p2v.XXXXXX/physical.xml"; char wrapper_script[] = "/tmp/p2v.XXXXXX/virt-v2v-wrapper.sh"; char dmesg_file[] = "/tmp/p2v.XXXXXX/dmesg"; char lscpu_file[] = "/tmp/p2v.XXXXXX/lscpu"; char lspci_file[] = "/tmp/p2v.XXXXXX/lspci"; char lsscsi_file[] = "/tmp/p2v.XXXXXX/lsscsi"; char lsusb_file[] = "/tmp/p2v.XXXXXX/lsusb"; char p2v_version_file[] = "/tmp/p2v.XXXXXX/p2v-version"; int inhibit_fd = -1; #if DEBUG_STDERR print_config (config, stderr); fprintf (stderr, "\n"); #endif set_control_h (NULL); set_running (1); set_cancel_requested (0); inhibit_fd = inhibit_power_saving (); #ifdef DEBUG_STDERR if (inhibit_fd == -1) fprintf (stderr, "warning: virt-p2v cannot inhibit power saving during conversion.\n"); #endif data_conns = malloc (sizeof (struct data_conn) * nr_disks); if (data_conns == NULL) error (EXIT_FAILURE, errno, "malloc"); for (i = 0; config->disks[i] != NULL; ++i) { data_conns[i].h = NULL; - data_conns[i].nbd_pid = 0; data_conns[i].nbd_remote_port = -1; } /* Start the data connections and NBD server processes, one per disk. */ for (i = 0; config->disks[i] != NULL; ++i) { int nbd_local_port; CLEANUP_FREE char *device = NULL; + pid_t nbd_pid; if (config->disks[i][0] == '/') { device = strdup (config->disks[i]); if (!device) { perror ("strdup"); cleanup_data_conns (data_conns, nr_disks); exit (EXIT_FAILURE); } } else if (asprintf (&device, "/dev/%s", config->disks[i]) == -1) { perror ("asprintf"); cleanup_data_conns (data_conns, nr_disks); exit (EXIT_FAILURE); } if (notify_ui) { CLEANUP_FREE char *msg; if (asprintf (&msg, _("Starting local NBD server for %s ..."), config->disks[i]) == -1) error (EXIT_FAILURE, errno, "asprintf"); notify_ui (NOTIFY_STATUS, msg); } /* Start NBD server listening on the given port number. */ - data_conns[i].nbd_pid = start_nbd_server (&nbd_local_port, device); - if (data_conns[i].nbd_pid == 0) { + nbd_pid = start_nbd_server (&nbd_local_port, device); + if (nbd_pid == 0) { set_conversion_error ("NBD server error: %s", get_nbd_error ()); goto out; } /* Wait for NBD server to start up and listen. */ if (wait_for_nbd_server_to_start (nbd_local_port) == -1) { set_conversion_error ("NBD server error: %s", get_nbd_error ()); goto out; } if (notify_ui) { CLEANUP_FREE char *msg; if (asprintf (&msg, _("Opening data connection for %s ..."), config->disks[i]) == -1) error (EXIT_FAILURE, errno, "asprintf"); notify_ui (NOTIFY_STATUS, msg); } /* Open the SSH data connection, with reverse port forwarding * back to the NBD server. */ data_conns[i].h = open_data_connection (config, nbd_local_port, &data_conns[i].nbd_remote_port); if (data_conns[i].h == NULL) { const char *err = get_ssh_error (); set_conversion_error ("could not open data connection over SSH to the conversion server: %s", err); goto out; } #if DEBUG_STDERR fprintf (stderr, "%s: data connection for %s: SSH remote port %d, local port %d\n", g_get_prgname (), device, data_conns[i].nbd_remote_port, nbd_local_port); #endif } /* Create a remote directory name which will be used for libvirt * XML, log files and other stuff. We don't delete this directory * after the run because (a) it's useful for debugging and (b) it * only contains small files. * * NB: This path MUST NOT require shell quoting. */ time (&now); gmtime_r (&now, &tm); if (asprintf (&remote_dir, "/tmp/virt-p2v-%04d%02d%02d-XXXXXXXX", tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday) == -1) { perror ("asprintf"); cleanup_data_conns (data_conns, nr_disks); exit (EXIT_FAILURE); } len = strlen (remote_dir); guestfs_int_random_string (&remote_dir[len-8], 8); if (notify_ui) notify_ui (NOTIFY_LOG_DIR, remote_dir); /* Generate the local temporary directory. */ if (mkdtemp (tmpdir) == NULL) { perror ("mkdtemp"); cleanup_data_conns (data_conns, nr_disks); exit (EXIT_FAILURE); } memcpy (name_file, tmpdir, strlen (tmpdir)); memcpy (physical_xml_file, tmpdir, strlen (tmpdir)); memcpy (wrapper_script, tmpdir, strlen (tmpdir)); memcpy (dmesg_file, tmpdir, strlen (tmpdir)); memcpy (lscpu_file, tmpdir, strlen (tmpdir)); memcpy (lspci_file, tmpdir, strlen (tmpdir)); memcpy (lsscsi_file, tmpdir, strlen (tmpdir)); memcpy (lsusb_file, tmpdir, strlen (tmpdir)); memcpy (p2v_version_file, tmpdir, strlen (tmpdir)); /* Generate the static files. */ generate_name (config, name_file); generate_physical_xml (config, data_conns, physical_xml_file); generate_wrapper_script (config, remote_dir, wrapper_script); generate_system_data (dmesg_file, lscpu_file, lspci_file, lsscsi_file, lsusb_file); generate_p2v_version_file (p2v_version_file); /* Open the control connection. This also creates remote_dir. */ if (notify_ui) notify_ui (NOTIFY_STATUS, _("Setting up the control connection ...")); set_control_h (start_remote_connection (config, remote_dir)); if (control_h == NULL) { set_conversion_error ("could not open control connection over SSH to the conversion server: %s", get_ssh_error ()); goto out; } /* Copy the static files to the remote dir. */ /* These three files must not fail, so check for errors here. */ if (scp_file (config, remote_dir, name_file, physical_xml_file, wrapper_script, NULL) == -1) { set_conversion_error ("scp: %s: %s", remote_dir, get_ssh_error ()); goto out; } /* It's not essential that these files are copied, so ignore errors. */ ignore_value (scp_file (config, remote_dir, dmesg_file, lscpu_file, lspci_file, lsscsi_file, lsusb_file, p2v_version_file, NULL)); /* Do the conversion. This runs until virt-v2v exits. */ if (notify_ui) notify_ui (NOTIFY_STATUS, _("Doing conversion ...")); if (mexp_printf (control_h, /* To simplify things in the wrapper script, it * writes virt-v2v's exit status to * /remote_dir/status, and here we read that and * exit the ssh shell with the same status. */ "%s/virt-v2v-wrapper.sh; " "exit $(< %s/status)\n", remote_dir, remote_dir) == -1) { set_conversion_error ("mexp_printf: virt-v2v: %m"); goto out; } /* Read output from the virt-v2v process and echo it through the * notify function, until virt-v2v closes the connection. */ while (!is_cancel_requested ()) { char buf[257]; ssize_t r; r = read (mexp_get_fd (control_h), buf, sizeof buf - 1); if (r == -1) { /* See comment about this in miniexpect.c. */ if (errno == EIO) break; /* EOF */ set_conversion_error ("read: %m"); goto out; } if (r == 0) break; /* EOF */ buf[r] = '\0'; if (notify_ui) notify_ui (NOTIFY_REMOTE_MESSAGE, buf); } if (is_cancel_requested ()) { set_conversion_error ("cancelled by user"); if (notify_ui) notify_ui (NOTIFY_STATUS, _("Conversion cancelled by user.")); goto out; } if (notify_ui) notify_ui (NOTIFY_STATUS, _("Control connection closed by remote.")); ret = 0; out: if (control_h) { mexp_h *h = control_h; set_control_h (NULL); status = mexp_close (h); if (status == -1) { set_conversion_error ("mexp_close: %m"); ret = -1; } else if (ret == 0 && WIFEXITED (status) && WEXITSTATUS (status) != 0) { set_conversion_error ("virt-v2v exited with status %d", WEXITSTATUS (status)); ret = -1; } } cleanup_data_conns (data_conns, nr_disks); if (inhibit_fd >= 0) close (inhibit_fd); set_running (0); return ret; } @@ -436,25 +436,19 @@ static void cleanup_data_conns (struct data_conn *data_conns, size_t nr) { size_t i; for (i = 0; i < nr; ++i) { if (data_conns[i].h != NULL) { /* Because there is no SSH prompt (ssh -N), the only way to kill * these ssh connections is to send a signal. Just closing the * pipe doesn't do anything. */ kill (mexp_get_pid (data_conns[i].h), SIGHUP); mexp_close (data_conns[i].h); } - - if (data_conns[i].nbd_pid > 0) { - /* Kill NBD process and clean up. */ - kill (data_conns[i].nbd_pid, SIGTERM); - waitpid (data_conns[i].nbd_pid, NULL, 0); - } } } /** * Write the guest name into C<filename>. */ diff --git a/nbd.c b/nbd.c index e30b4a2195cb..dcedd0a52dce 100644 --- a/nbd.c +++ b/nbd.c @@ -187,50 +187,51 @@ static pid_t start_nbdkit (const char *device, int *fds, size_t nr_fds) { pid_t pid; CLEANUP_FREE char *file_str = NULL; #if DEBUG_STDERR fprintf (stderr, "starting nbdkit for %s using socket activation\n", device); #endif if (asprintf (&file_str, "file=%s", device) == -1) error (EXIT_FAILURE, errno, "asprintf"); pid = fork (); if (pid == -1) { set_nbd_error ("fork: %m"); return 0; } if (pid == 0) { /* Child. */ close (0); if (open ("/dev/null", O_RDONLY) == -1) { perror ("open: /dev/null"); _exit (EXIT_FAILURE); } socket_activation (fds, nr_fds); execlp ("nbdkit", "nbdkit", - "-r", /* readonly (vital!) */ - "-f", /* don't fork */ - "file", /* file plugin */ - file_str, /* a device like file=/dev/sda */ + "-r", /* readonly (vital!) */ + "--exit-with-parent", /* don't fork, and exit when the parent thread + * does */ + "file", /* file plugin */ + file_str, /* a device like file=/dev/sda */ NULL); perror ("nbdkit"); _exit (EXIT_FAILURE); } /* Parent. */ return pid; } /** * Open a listening socket on an unused local port and return it. * * Returns the port number on success or C<-1> on error. * * The file descriptor(s) bound are returned in the array *fds, *nr_fds. * The caller must free the array. */ diff --git a/p2v.h b/p2v.h index c55f64317dde..0846a2f959fa 100644 --- a/p2v.h +++ b/p2v.h @@ -85,7 +85,6 @@ extern void gui_conversion (struct config *); /* conversion.c */ struct data_conn { /* Data per NBD connection / physical disk. */ mexp_h *h; /* miniexpect handle to ssh */ - pid_t nbd_pid; /* NBD server PID */ int nbd_remote_port; /* remote NBD port on conversion server */ }; -- 2.19.1.3.g30247aa5d201

(This patch is similar to nbdkit commit 9eec2335d630, "server/sockets: get rid of AI_ADDRCONFIG", 2022-01-19). Consider the following call tree: start_conversion() [conversion.c] start_nbd_server() [nbd.c] open_listening_socket() [nbd.c] bind_tcpip_socket() [nbd.c] getaddrinfo() socket() bind() wait_for_nbd_server_to_start() [nbd.c] connect_to_nbdkit() [nbd.c] getaddrinfo() socket() connect() open_data_connection() [ssh.c] /* "-R 0:localhost:<port>" */ - For each of IPv4 and IPv6, if the network config on the host running virt-p2v supports that protocol, then bind_tcpip_socket() intends to bind the port for that protocol. - connect_to_nbdkit() connects to the port using *one* of IPv4 and IPv6; it just wants to see NBDMAGIC, regardless of IP version. - The ssh "-R 0:localhost:<port>" option, formatted by open_data_connection(), instructs ssh to create a reverse forwarding channel (a listening socket) per IP version (this can be verified with "netstat" on the conversion server). In case the reverse NBD connection on the conversion server were made to sshd over IPv6, then ssh on the p2v server would presumably want to connect to nbdkit over IPv6 too. The (theoretical) problem with using AI_ADDRCONFIG in bind_tcpip_socket() is that, in case the p2v server has no publicly routable IPv6 address assigned, then bind_tcpip_socket() will not bind ::1 from "localhost". And then the IPv6 reverse forwarding attempt, set up by open_data_connection(), *might* fail. Remove AI_ADDRCONFIG anyway. While at it, spell out AF_UNSPEC as well (in practice, this makes no difference, as Linux defines AF_UNSPEC as PF_UNSPEC as 0). While running the local test suite ("make -j10 check") on a host without a publicly routable IPv6 address, the "netstat -anp" output changes, due to this patch. Before: We have two nbdkit processes (PIDs 51057 and 51050) listening over TCPv4, and a third one (PID 51204) connected to PID 51050 over TCPv4. After: The two listening nbdkit processes (PIDs 49310 and 49303) are now doing so over both TCPv4 and TCPv6, and the third one (PID 49457) actually connects to PID 49303 over TCPv6! Ref: https://listman.redhat.com/archives/libguestfs/2022-March/028475.html Suggested-by: Richard W.M. Jones <rjones(a)redhat.com&gt; Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- nbd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nbd.c b/nbd.c index dcedd0a52dce..92fad34c0e32 100644 --- a/nbd.c +++ b/nbd.c @@ -262,90 +262,91 @@ static int bind_tcpip_socket (const char *port, int **fds_rtn, size_t *nr_fds_rtn) { struct addrinfo *ai = NULL; struct addrinfo hints; struct addrinfo *a; int err; int *fds = NULL; size_t nr_fds; int addr_in_use = 0; memset (&hints, 0, sizeof hints); - hints.ai_flags = AI_PASSIVE | AI_ADDRCONFIG; + hints.ai_family = AF_UNSPEC; + hints.ai_flags = AI_PASSIVE; hints.ai_socktype = SOCK_STREAM; err = getaddrinfo ("localhost", port, &hints, &ai); if (err != 0) { #if DEBUG_STDERR fprintf (stderr, "%s: getaddrinfo: localhost: %s: %s", g_get_prgname (), port, gai_strerror (err)); #endif return -1; } nr_fds = 0; for (a = ai; a != NULL; a = a->ai_next) { int sock, opt; sock = socket (a->ai_family, a->ai_socktype, a->ai_protocol); if (sock == -1) error (EXIT_FAILURE, errno, "socket"); opt = 1; if (setsockopt (sock, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof opt) == -1) perror ("setsockopt: SO_REUSEADDR"); #ifdef IPV6_V6ONLY if (a->ai_family == PF_INET6) { if (setsockopt (sock, IPPROTO_IPV6, IPV6_V6ONLY, &opt, sizeof opt) == -1) perror ("setsockopt: IPv6 only"); } #endif if (bind (sock, a->ai_addr, a->ai_addrlen) == -1) { if (errno == EADDRINUSE) { addr_in_use = 1; close (sock); continue; } perror ("bind"); close (sock); continue; } if (listen (sock, SOMAXCONN) == -1) { perror ("listen"); close (sock); continue; } nr_fds++; fds = realloc (fds, sizeof (int) * nr_fds); if (!fds) error (EXIT_FAILURE, errno, "realloc"); fds[nr_fds-1] = sock; } freeaddrinfo (ai); if (nr_fds == 0 && addr_in_use) { #if DEBUG_STDERR fprintf (stderr, "%s: unable to bind to localhost:%s: %s\n", g_get_prgname (), port, strerror (EADDRINUSE)); #endif return -1; } #if DEBUG_STDERR fprintf (stderr, "%s: bound to localhost:%s (%zu socket(s))\n", g_get_prgname (), port, nr_fds); #endif *fds_rtn = fds; *nr_fds_rtn = nr_fds; return 0; } /** * Wait for nbdkit to start and be listening for connections. */ -- 2.19.1.3.g30247aa5d201

On 03/31/22 15:14, Richard W.M. Jones wrote: Thanks! BTW: do you have any comments about the following serial log message (see the cover letter): I did not check if the same was issued *without* this patch in place; I highlighted it in the cover letter because it could ring a bell with you (regardless of AI_ADDRCONFIG / IPv6). Thanks! Laszlo