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. The package references for the various distros, in "dependencies.m4", come from: - https://packages.debian.org/sid/amd64/nbdkit/filelist - https://aur.archlinux.org/packages/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; --- Notes: v2: - update package names for nbdkit, extend commit message [Rich] - keep the skip check for nbdkit in "test-virt-p2v-nbdkit.sh" [Rich] Makefile.am | 2 - main.c | 2 +- nbd.c | 136 +------------------- 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 | 13 +- docs/p2v-building.pod | 9 +- test-virt-p2v-nbdkit.sh | 4 +- test-virt-p2v-scp.sh | 2 +- test-virt-p2v-ssh.sh | 4 +- test-virt-p2v.sh | 56 -------- virt-p2v.pod | 29 ++--- 12 files changed, 35 insertions(+), 263 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/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/contrib/aux-scripts/do-build.sh b/contrib/aux-scripts/do-build.sh index 53a844d9c99d..4a0aac840df0 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,^nbdkit-server.*,,g' \ + -e 's,^nbdkit-file-plugin.*,,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,^nbdkit-server.*,,g' \ + -e 's,^nbdkit-file-plugin.*,,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..fc3715e4d3c7 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 + nbdkit-server + nbdkit-file-plugin 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 ) @@ -63,19 +64,19 @@ ifelse(REDHAT,1, ifelse(DEBIAN,1, libpcre3 libxml2 ifelse(GTK_VERSION,2,libgtk`'GTK_VERSION`'.0-0,libgtk-`'GTK_VERSION`'-0) libdbus-1-3 openssh-client - qemu-utils + nbdkit debianutils vim-tiny open-iscsi xorg xserver-xorg-video-all fonts-dejavu metacity network-manager network-manager-gnome dbus-x11 net-tools ) @@ -83,21 +84,21 @@ ifelse(DEBIAN,1, ifelse(ARCHLINUX,1, pcre libxml2 gtk`'GTK_VERSION dbus openssh - qemu + nbdkit which vim-tiny open-iscsi xorg-xinit xorg-server xf86-video-* ttf-dejavu metacity NetworkManager nm-connection-editor network-manager-applet dbus-x11 net-tools ) @@ -105,22 +106,23 @@ ifelse(ARCHLINUX,1, ifelse(SUSE,1, pcre libxml2 gtk`'GTK_VERSION libdbus-1-3 - qemu-tools + nbdkit-server + nbdkit-file-plugin openssh dnl /usr/bin/which is in util-linux on SUSE vim open-iscsi xinit xorg-x11-server xf86-video-* dejavu-fonts NetworkManager xf86-input-* icewm-lite dbus-1-x11 yast2-network libyui-qt SuSEfirewall2 ) @@ -128,33 +130,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 + nbdkit-server + nbdkit-file-plugin 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/test-virt-p2v-nbdkit.sh b/test-virt-p2v-nbdkit.sh index 9adb195f9249..bba4bda4ab87 100755 --- a/test-virt-p2v-nbdkit.sh +++ b/test-virt-p2v-nbdkit.sh @@ -1,21 +1,21 @@ #!/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 @@ -47,7 +47,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

According to the first commit message in this series, remove support for parsing "--nbd=nbdkit-no-sa". Systemd socket activation was added to nbdkit in nbdkit commit 7ff39d028c63 ("Add support for socket activation.", 2017-01-31); first released in nbdkit v1.1.13 (Aug 2017). 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; Reviewed-by: Richard W.M. Jones <rjones(a)redhat.com&gt; --- Notes: v2: - in the commit message, highlight the nbdkit commit that introduced systemd socket activation [Rich] - pick up Rich's R-b 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 bba4bda4ab87..0b020cfe4bed 100755 --- a/test-virt-p2v-nbdkit.sh +++ b/test-virt-p2v-nbdkit.sh @@ -48,7 +48,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

At this point, the only NBD server type virt-p2v supports is nbdkit with socket activation; therefore, outputting "localhost" from start_nbd_server(), and tracking it via variables / parameters from there on is needless generality. Remove the affected variables and parameters, such as "ipaddr", "hostname", and "nbd_local_ipaddr". 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; Reviewed-by: Richard W.M. Jones <rjones(a)redhat.com&gt; --- Notes: v2: - pick up Rich's R-b p2v.h | 6 +-- conversion.c | 13 ++--- nbd.c | 51 +++++++++----------- ssh.c | 6 +-- 4 files changed, 33 insertions(+), 43 deletions(-) diff --git a/p2v.h b/p2v.h index a14edc52ef74..4d2d20648467 100644 --- a/p2v.h +++ b/p2v.h @@ -105,7 +105,7 @@ extern int inhibit_power_saving (void); /* ssh.c */ extern int test_connection (struct config *); -extern mexp_h *open_data_connection (struct config *, const char *local_ipaddr, int local_port, int *remote_port); +extern mexp_h *open_data_connection (struct config *, int local_port, int *remote_port); extern mexp_h *start_remote_connection (struct config *, const char *remote_dir); extern const char *get_ssh_error (void); extern int scp_file (struct config *config, const char *target, const char *local, ...) __attribute__((sentinel)); @@ -113,8 +113,8 @@ extern int scp_file (struct config *config, const char *target, const char *loca /* nbd.c */ extern void set_nbd_option (const char *opt); extern void test_nbd_servers (void); -extern pid_t start_nbd_server (const char **ipaddr, int *port, const char *device); -extern int wait_for_nbd_server_to_start (const char *ipaddr, int port); +extern pid_t start_nbd_server (int *port, const char *device); +extern int wait_for_nbd_server_to_start (int port); const char *get_nbd_error (void); /* utils.c */ diff --git a/conversion.c b/conversion.c index 8daa1d9f39fa..8bc4119fb47c 100644 --- a/conversion.c +++ b/conversion.c @@ -155,270 +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) { - const char *nbd_local_ipaddr; int nbd_local_port; CLEANUP_FREE char *device = NULL; 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_ipaddr, &nbd_local_port, device); + data_conns[i].nbd_pid = start_nbd_server (&nbd_local_port, device); if (data_conns[i].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_ipaddr, nbd_local_port) == -1) { + 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_ipaddr, nbd_local_port, + 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 %s:%d\n", + "%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_ipaddr, nbd_local_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; } diff --git a/nbd.c b/nbd.c index cda962d03bdd..cb849e75dc84 100644 --- a/nbd.c +++ b/nbd.c @@ -81,9 +81,9 @@ static const enum nbd_server standard_servers[] = 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 (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); +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 char *nbd_error; @@ -235,32 +235,31 @@ test_nbd_servers (void) * Returns the process ID (E<gt> 0) or C<0> if there is an error. */ pid_t -start_nbd_server (const char **ipaddr, int *port, const char *device) +start_nbd_server (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); + *port = open_listening_socket (&fds, &nr_fds); if (*port == -1) return -1; pid = start_nbdkit (device, fds, nr_fds); for (i = 0; i < nr_fds; ++i) close (fds[i]); free (fds); return pid; } 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. */ @@ -350,261 +349,257 @@ start_nbdkit (const char *device, int *fds, size_t nr_fds) * The caller must free the array. */ static int -open_listening_socket (const char *ipaddr, int **fds, size_t *nr_fds) +open_listening_socket (int **fds, size_t *nr_fds) { int port; char port_str[16]; /* This just ensures we don't try the port we previously bound to. */ port = nbd_local_port; /* Search for a free port. */ for (; port < 60000; ++port) { snprintf (port_str, sizeof port_str, "%d", port); - if (bind_tcpip_socket (ipaddr, port_str, fds, nr_fds) == 0) { + if (bind_tcpip_socket (port_str, fds, nr_fds) == 0) { /* See above. */ nbd_local_port = port + 1; return port; } } set_nbd_error ("cannot find a free local port"); return -1; } static int -bind_tcpip_socket (const char *ipaddr, const char *port, - int **fds_rtn, size_t *nr_fds_rtn) +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_socktype = SOCK_STREAM; - err = getaddrinfo (ipaddr, port, &hints, &ai); + err = getaddrinfo ("localhost", port, &hints, &ai); if (err != 0) { #if DEBUG_STDERR - fprintf (stderr, "%s: getaddrinfo: %s: %s: %s", - g_get_prgname (), ipaddr ? ipaddr : "<any>", port, - gai_strerror (err)); + 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 %s:%s: %s\n", - g_get_prgname (), ipaddr ? ipaddr : "<any>", port, - strerror (EADDRINUSE)); + 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 IP address %s:%s (%zu socket(s))\n", - g_get_prgname (), ipaddr ? ipaddr : "<any>", port, nr_fds); + 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 a local NBD server to start and be listening for * connections. */ int -wait_for_nbd_server_to_start (const char *ipaddr, int port) +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 (ipaddr, port, port+1); + sockfd = connect_with_source_port (port, port+1); 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<hostname:dest_port>, resolving the address using + * 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 (const char *hostname, int dest_port, int source_port) +connect_with_source_port (int dest_port, int source_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 (hostname, dest_port_str, &hints, &results); + r = getaddrinfo ("localhost", dest_port_str, &hints, &results); if (r != 0) { - set_nbd_error ("getaddrinfo: %s/%s: %s", - hostname, dest_port_str, gai_strerror (r)); + 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 %s/%s: %m", - hostname, dest_port_str); + "connect to localhost/%s: %m", dest_port_str); close (sockfd); sockfd = -1; continue; } break; } freeaddrinfo (results); return sockfd; } diff --git a/ssh.c b/ssh.c index 42701433f0cc..5e1d60b8a2e7 100644 --- a/ssh.c +++ b/ssh.c @@ -1062,69 +1062,67 @@ compatible_version (const char *v2v_version) } mexp_h * -open_data_connection (struct config *config, - const char *local_ipaddr, int local_port, - int *remote_port) +open_data_connection (struct config *config, int local_port, int *remote_port) { mexp_h *h; char remote_arg[32]; const char *extra_args[] = { "-R", remote_arg, "-N", NULL }; CLEANUP_FREE char *port_str = NULL; const int ovecsize = 12; int ovector[ovecsize]; - snprintf (remote_arg, sizeof remote_arg, "0:%s:%d", local_ipaddr, local_port); + snprintf (remote_arg, sizeof remote_arg, "0:localhost:%d", local_port); h = start_ssh (0, config, (char **) extra_args, 0); if (h == NULL) return NULL; switch (mexp_expect (h, (mexp_regexp[]) { { 100, .re = portfwd_re }, { 0 } }, ovector, ovecsize)) { case 100: /* Ephemeral port. */ port_str = strndup (&h->buffer[ovector[2]], ovector[3]-ovector[2]); if (port_str == NULL) { set_ssh_internal_error ("strndup: %m"); mexp_close (h); return NULL; } if (sscanf (port_str, "%d", remote_port) != 1) { set_ssh_internal_error ("cannot extract the port number from '%s'", port_str); mexp_close (h); return NULL; } break; case MEXP_EOF: set_ssh_unexpected_eof ("\"ssh -R\" output"); mexp_close (h); return NULL; case MEXP_TIMEOUT: set_ssh_unexpected_timeout ("\"ssh -R\" output"); mexp_close (h); return NULL; case MEXP_ERROR: set_ssh_mexp_error ("mexp_expect"); mexp_close (h); return NULL; case MEXP_PCRE_ERROR: set_ssh_pcre_error (); mexp_close (h); return NULL; } return h; } /* Wait for the prompt. */ -- 2.19.1.3.g30247aa5d201

Remove the --nbd command line option, its documentation, the parser function set_nbd_option(), and the internal representation "cmdline_servers". Unconditionally use the "standard_servers" array (which now consists of the NBDKIT server type only). Further patches will simplify the remaining code. 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; Reviewed-by: Richard W.M. Jones <rjones(a)redhat.com&gt; --- Notes: v2: - pick up Rich's R-b p2v.h | 1 - main.c | 5 -- nbd.c | 64 +++----------------- test-virt-p2v-nbdkit.sh | 5 +- virt-p2v.pod | 14 ----- 5 files changed, 10 insertions(+), 79 deletions(-) diff --git a/p2v.h b/p2v.h index 4d2d20648467..3093e64ca3d7 100644 --- a/p2v.h +++ b/p2v.h @@ -111,7 +111,6 @@ extern const char *get_ssh_error (void); extern int scp_file (struct config *config, const char *target, const char *local, ...) __attribute__((sentinel)); /* nbd.c */ -extern void set_nbd_option (const char *opt); extern void test_nbd_servers (void); extern pid_t start_nbd_server (int *port, const char *device); extern int wait_for_nbd_server_to_start (int port); diff --git a/main.c b/main.c index e4e9c1fe21c1..dc411b733d31 100644 --- a/main.c +++ b/main.c @@ -69,16 +69,15 @@ static const char options[] = "Vv"; static const struct option long_options[] = { { "help", 0, 0, HELP_OPTION }, { "cmdline", 1, 0, 0 }, { "color", 0, 0, 0 }, { "colors", 0, 0, 0 }, { "colour", 0, 0, 0 }, { "colours", 0, 0, 0 }, { "iso", 0, 0, 0 }, - { "nbd", 1, 0, 0 }, { "long-options", 0, 0, 0 }, { "short-options", 0, 0, 0 }, { "test-disk", 1, 0, 0 }, { "verbose", 0, 0, 'v' }, { "version", 0, 0, 'V' }, { 0, 0, 0, 0 } }; @@ -87,27 +86,26 @@ 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=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. */ @@ -137,128 +135,125 @@ int main (int argc, char *argv[]) { gboolean gui_possible; int c; int option_index; char **cmdline = NULL; int cmdline_source = 0; struct config *config = new_config (); setlocale (LC_ALL, ""); bindtextdomain (PACKAGE, LOCALEBASEDIR); textdomain (PACKAGE); /* We may use random(3) in this program. */ srandom (time (NULL) + getpid ()); /* There is some raciness between slow devices being discovered by * the kernel and udev and virt-p2v running. This is a partial * workaround, but a real fix involves handling hotplug events * (possible in GUI mode, not easy in kernel mode). */ udevadm_settle (); gui_possible = gtk_init_check (&argc, &argv); for (;;) { c = getopt_long (argc, argv, options, long_options, &option_index); if (c == -1) break; switch (c) { case 0: /* options which are long only */ if (STREQ (long_options[option_index].name, "long-options")) { display_long_options (long_options); } else if (STREQ (long_options[option_index].name, "short-options")) { display_short_options (options); } else if (STREQ (long_options[option_index].name, "cmdline")) { cmdline = parse_cmdline_string (optarg); cmdline_source = CMDLINE_SOURCE_COMMAND_LINE; } else if (STREQ (long_options[option_index].name, "color") || STREQ (long_options[option_index].name, "colour") || STREQ (long_options[option_index].name, "colors") || STREQ (long_options[option_index].name, "colours")) { force_colour = 1; } else if (STREQ (long_options[option_index].name, "iso")) { is_iso_environment = 1; } - else if (STREQ (long_options[option_index].name, "nbd")) { - set_nbd_option (optarg); /* in nbd.c */ - } else if (STREQ (long_options[option_index].name, "test-disk")) { if (test_disk != NULL) error (EXIT_FAILURE, 0, _("only a single --test-disk option can be used")); if (optarg[0] != '/') error (EXIT_FAILURE, 0, _("--test-disk must be an absolute path")); test_disk = optarg; } else error (EXIT_FAILURE, 0, _("unknown long option: %s (%d)"), long_options[option_index].name, option_index); break; case 'v': /* This option does nothing since 1.33.41. Verbose is always * enabled. */ break; case 'V': printf ("%s %s\n", g_get_prgname (), PACKAGE_VERSION_FULL); exit (EXIT_SUCCESS); case HELP_OPTION: usage (EXIT_SUCCESS); default: usage (EXIT_FAILURE); } } if (optind != argc) { fprintf (stderr, _("%s: unused arguments on the command line\n"), g_get_prgname ()); usage (EXIT_FAILURE); } test_nbd_servers (); set_config_defaults (config); /* Parse /proc/cmdline (if it exists) or use the --cmdline parameter * to initialize the configuration. This allows defaults to be pass * using the kernel command line, with additional GUI configuration * later. */ if (cmdline == NULL) { cmdline = parse_proc_cmdline (); if (cmdline != NULL) cmdline_source = CMDLINE_SOURCE_PROC_CMDLINE; } if (cmdline) update_config_from_kernel_cmdline (config, cmdline); /* If p2v.server exists, then we use the non-interactive kernel * conversion. Otherwise we run the GUI. */ if (config->remote.server != NULL) kernel_conversion (config, cmdline, cmdline_source); else { if (!gui_possible) error (EXIT_FAILURE, 0, _("gtk_init_check returned false, indicating that\n" "a GUI is not possible on this host. Check X11, $DISPLAY etc.")); gui_conversion (config); } guestfs_int_free_string_list (cmdline); free_config (config); exit (EXIT_SUCCESS); } diff --git a/nbd.c b/nbd.c index f64afdc53dd3..7d8abf3cbbb0 100644 --- a/nbd.c +++ b/nbd.c @@ -1,76 +1,72 @@ /* 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 L<nbdkit(1)>. - */ +/* This file handles 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). */ static int nbd_local_port; -/* List of servers specified by the --nbd option. */ +/* Supported server types. */ enum nbd_server { /* 0 is reserved for "end of list" */ NBDKIT = 1, }; -static enum nbd_server *cmdline_servers = NULL; 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; } -/* If no --nbd option is passed, we use this standard list instead. - * Must match the documentation in virt-p2v(1). +/* We use this standard list of nbd server types. Must match the documentation + * in virt-p2v(1). */ static const enum nbd_server standard_servers[] = { NBDKIT, 0 }; @@ -113,47 +109,11 @@ const char * get_nbd_error (void) { return nbd_error; } /** - * The main program calls this to set the I<--nbd> option. - */ -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 - 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. + * Test the built-in default list to see which servers are actually installed + * and appear to be working. * * Set the C<use_server> global accordingly. */ @@ -161,75 +121,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); - if (cmdline_servers != NULL) - servers = cmdline_servers; - else - servers = standard_servers; + 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 ); 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"), + _("%s: no working NBD server was found, cannot continue.\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. */ diff --git a/test-virt-p2v-nbdkit.sh b/test-virt-p2v-nbdkit.sh index 0b020cfe4bed..510b9d646ad1 100755 --- a/test-virt-p2v-nbdkit.sh +++ b/test-virt-p2v-nbdkit.sh @@ -1,21 +1,21 @@ #!/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. +# Test virt-p2v in non-GUI mode. set -e @@ -47,8 +47,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. -$VG virt-p2v --cmdline="$cmdline" --nbd=nbdkit +$VG virt-p2v --cmdline="$cmdline" # 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 e6cbb1514edc..09d3c7729e38 100644 --- a/virt-p2v.pod +++ b/virt-p2v.pod @@ -506,20 +506,6 @@ virt-p2v ISO environment, ie. when it is running on a real physical machine (and thus not when testing). It enables various dangerous 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>. - -=over 4 - -=item B<nbdkit> - -Use nbdkit with the file plugin (see: L<nbdkit-file-plugin(1)>) and -socket activation. - -=back - =item B<--test-disk=/PATH/TO/DISK.IMG> For testing or debugging purposes, replace F</dev/sda> with a local -- 2.19.1.3.g30247aa5d201

Remove the "enum nbd_server" type, and the "standard_servers" and "use_server" variables, as no real selection is happening now. With regard to naming: internally to "nbd.c", and in the user-visible documentation, replace (or restrict) "NBD server" references with "nbdkit". In source code different from "nbd.c" however, stick with the "NBD server" language. Rename test_nbd_servers() (plural) to test_nbd_server() (singular). This patch is best viewed with "git show -b"; otherwise, the unindentations due to the switch statements' removal obscure the diff somewhat. 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; Reviewed-by: Richard W.M. Jones <rjones(a)redhat.com&gt; --- Notes: v2: - pick up Rich's R-b p2v.h | 2 +- main.c | 2 +- nbd.c | 110 ++++++-------------- virt-p2v.pod | 6 +- 4 files changed, 34 insertions(+), 86 deletions(-) diff --git a/p2v.h b/p2v.h index 3093e64ca3d7..c55f64317dde 100644 --- a/p2v.h +++ b/p2v.h @@ -111,7 +111,7 @@ extern const char *get_ssh_error (void); extern int scp_file (struct config *config, const char *target, const char *local, ...) __attribute__((sentinel)); /* nbd.c */ -extern void test_nbd_servers (void); +extern void test_nbd_server (void); extern pid_t start_nbd_server (int *port, const char *device); extern int wait_for_nbd_server_to_start (int port); const char *get_nbd_error (void); diff --git a/main.c b/main.c index dc411b733d31..0ebb7291c7ce 100644 --- a/main.c +++ b/main.c @@ -135,125 +135,125 @@ int main (int argc, char *argv[]) { gboolean gui_possible; int c; int option_index; char **cmdline = NULL; int cmdline_source = 0; struct config *config = new_config (); setlocale (LC_ALL, ""); bindtextdomain (PACKAGE, LOCALEBASEDIR); textdomain (PACKAGE); /* We may use random(3) in this program. */ srandom (time (NULL) + getpid ()); /* There is some raciness between slow devices being discovered by * the kernel and udev and virt-p2v running. This is a partial * workaround, but a real fix involves handling hotplug events * (possible in GUI mode, not easy in kernel mode). */ udevadm_settle (); gui_possible = gtk_init_check (&argc, &argv); for (;;) { c = getopt_long (argc, argv, options, long_options, &option_index); if (c == -1) break; switch (c) { case 0: /* options which are long only */ if (STREQ (long_options[option_index].name, "long-options")) { display_long_options (long_options); } else if (STREQ (long_options[option_index].name, "short-options")) { display_short_options (options); } else if (STREQ (long_options[option_index].name, "cmdline")) { cmdline = parse_cmdline_string (optarg); cmdline_source = CMDLINE_SOURCE_COMMAND_LINE; } else if (STREQ (long_options[option_index].name, "color") || STREQ (long_options[option_index].name, "colour") || STREQ (long_options[option_index].name, "colors") || STREQ (long_options[option_index].name, "colours")) { force_colour = 1; } else if (STREQ (long_options[option_index].name, "iso")) { is_iso_environment = 1; } else if (STREQ (long_options[option_index].name, "test-disk")) { if (test_disk != NULL) error (EXIT_FAILURE, 0, _("only a single --test-disk option can be used")); if (optarg[0] != '/') error (EXIT_FAILURE, 0, _("--test-disk must be an absolute path")); test_disk = optarg; } else error (EXIT_FAILURE, 0, _("unknown long option: %s (%d)"), long_options[option_index].name, option_index); break; case 'v': /* This option does nothing since 1.33.41. Verbose is always * enabled. */ break; case 'V': printf ("%s %s\n", g_get_prgname (), PACKAGE_VERSION_FULL); exit (EXIT_SUCCESS); case HELP_OPTION: usage (EXIT_SUCCESS); default: usage (EXIT_FAILURE); } } if (optind != argc) { fprintf (stderr, _("%s: unused arguments on the command line\n"), g_get_prgname ()); usage (EXIT_FAILURE); } - test_nbd_servers (); + test_nbd_server (); set_config_defaults (config); /* Parse /proc/cmdline (if it exists) or use the --cmdline parameter * to initialize the configuration. This allows defaults to be pass * using the kernel command line, with additional GUI configuration * later. */ if (cmdline == NULL) { cmdline = parse_proc_cmdline (); if (cmdline != NULL) cmdline_source = CMDLINE_SOURCE_PROC_CMDLINE; } if (cmdline) update_config_from_kernel_cmdline (config, cmdline); /* If p2v.server exists, then we use the non-interactive kernel * conversion. Otherwise we run the GUI. */ if (config->remote.server != NULL) kernel_conversion (config, cmdline, cmdline_source); else { if (!gui_possible) error (EXIT_FAILURE, 0, _("gtk_init_check returned false, indicating that\n" "a GUI is not possible on this host. Check X11, $DISPLAY etc.")); gui_conversion (config); } guestfs_int_free_string_list (cmdline); free_config (config); exit (EXIT_SUCCESS); } diff --git a/nbd.c b/nbd.c index 48926d4aec1d..e30b4a2195cb 100644 --- a/nbd.c +++ b/nbd.c @@ -1,66 +1,49 @@ /* 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 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). */ +/* How long to wait for nbdkit to start (seconds). */ #define WAIT_NBD_TIMEOUT 10 -/* The local port that the NBD server listens on (incremented for - * each server which is started). +/* The local port that nbdkit listens on (incremented for each server which is + * started). */ static int nbd_local_port; -/* Supported server types. */ -enum nbd_server { - /* 0 is reserved for "end of list" */ - NBDKIT = 1, -}; - -/* We use this standard list of nbd server types. Must match the documentation - * in virt-p2v(1). - */ -static const enum nbd_server standard_servers[] = - { 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, 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); @@ -94,79 +77,53 @@ const char * get_nbd_error (void) { return nbd_error; } /** - * Test the built-in default list to see which servers are actually installed - * and appear to be working. - * - * Set the C<use_server> global accordingly. + * Check for nbdkit. */ void -test_nbd_servers (void) +test_nbd_server (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 nbdkit ...\n"); + fprintf (stderr, "checking for nbdkit ...\n"); #endif - switch (servers[i]) { - case NBDKIT: /* with socket activation */ - r = system ("nbdkit file --version" + r = system ("nbdkit file --version" #ifndef DEBUG_STDERR - " >/dev/null 2>&1" + " >/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"), + ); + if (r != 0) { + fprintf (stderr, _("%s: nbdkit was not found, cannot continue.\n"), g_get_prgname ()); exit (EXIT_FAILURE); } #if DEBUG_STDERR - fprintf (stderr, "picked nbdkit\n"); + fprintf (stderr, "found nbdkit\n"); #endif } /** - * Start the NBD server. + * Start nbdkit. * - * We previously tested all NBD servers (see C<test_nbd_servers>) and - * hopefully found one which will work. + * We previously tested nbdkit (see C<test_nbd_server>). * * Returns the process ID (E<gt> 0) or C<0> if there is an error. */ @@ -174,28 +131,23 @@ pid_t start_nbd_server (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 */ - *port = open_listening_socket (&fds, &nr_fds); - if (*port == -1) return -1; - pid = start_nbdkit (device, fds, nr_fds); - for (i = 0; i < nr_fds; ++i) - close (fds[i]); - free (fds); - return pid; - } - - abort (); + *port = open_listening_socket (&fds, &nr_fds); + if (*port == -1) return -1; + pid = start_nbdkit (device, fds, nr_fds); + for (i = 0; i < nr_fds; ++i) + close (fds[i]); + free (fds); + return pid; } #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. */ @@ -235,52 +187,50 @@ 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 */ NULL); perror ("nbdkit"); _exit (EXIT_FAILURE); } /* Parent. */ return pid; } /** - * 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. + * 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. */ @@ -311,160 +261,158 @@ 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_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 a local NBD server to start and be listening for - * connections. + * Wait for nbdkit to start and be listening for connections. */ 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"); + set_nbd_error ("timed out waiting for nbdkit to start"); goto cleanup; } 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"); + set_nbd_error ("waiting for nbdkit 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"); + set_nbd_error ("waiting for nbdkit 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"); + set_nbd_error ("waiting for nbdkit to start: " + "'NBDMAGIC' was not received from nbdkit"); 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 may involve multiple connections - to IPv4 and IPv6 for * instance. */ @@ -472,42 +420,42 @@ static int connect_to_nbdkit (int dest_port) { struct addrinfo hints; struct addrinfo *results, *rp; char dest_port_str[16]; int r, sockfd = -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; /* Connect. */ if (connect (sockfd, rp->ai_addr, rp->ai_addrlen) == -1) { - set_nbd_error ("waiting for NBD server to start: " + set_nbd_error ("waiting for nbdkit to start: " "connect to localhost/%s: %m", dest_port_str); close (sockfd); sockfd = -1; continue; } break; } freeaddrinfo (results); return sockfd; } diff --git a/virt-p2v.pod b/virt-p2v.pod index 09d3c7729e38..aec5078f92db 100644 --- a/virt-p2v.pod +++ b/virt-p2v.pod @@ -673,9 +673,9 @@ server and terminates on the conversion server, in fact NBD requests flow in the opposite direction. This is because the reverse port forward feature of ssh (C<ssh -R>) is used to open a port on the loopback interface of the conversion server which is proxied back by -ssh to the NBD server running on the physical machine. The effect is -that virt-v2v via libguestfs can open nbd connections which directly -read the hard disk(s) of the physical server. +ssh to nbdkit running on the physical machine. The effect is 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 nbdkit I<-r> (readonly) option is -- 2.19.1.3.g30247aa5d201

On Fri, Apr 01, 2022 at 12:28:06PM +0200, Laszlo Ersek wrote: I think this patch is reasonable and beneficial. 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 nbdkit - Flexible, fast NBD server with plugins https://gitlab.com/nbdkit/nbdkit

Consider the call tree again: start_conversion() [conversion.c] start_nbd_server() [nbd.c] open_listening_socket() [nbd.c] bind_tcpip_socket() [nbd.c] socket() bind() listen() start_nbdkit() [nbd.c] fork() /* child */ execlp() close() wait_for_nbd_server_to_start() [nbd.c] connect_to_nbdkit() [nbd.c] connect() open_data_connection() [ssh.c] /* "-R 0:localhost:<port>" */ Right after we fork(), inside (indirectly) start_nbd_server(), two file descriptors exist that point to the same file description that points to the same listening socket. In the child, execlp() preserves one of the file descriptors (that's the point of socket activation), while in the rest of start_nbd_server(), the parent close()s the other file descriptor. By the time the parent reaches connect(), there will have been no opportunity for the listening socket not to exist (unless, in the child, execlp() fails, or the new image exits very quickly). Therefore we don't need to test the child (by connecting to it and reading NBDMAGIC); we can just expose the listening socket to "ssh -R". Thus, remove the wait_for_nbd_server_to_start() and connect_to_nbdkit() functions. This eliminates the following -- harmless -- message during conversion, emitted by the nbdkit server child on the p2v machine, to the serial port: nbdkit complains because, after sending "NBDMAGIC", it would like to send "IHAVEOPT" and the handshake flags as well to the client. But the client -- namely wait_for_nbd_server_to_start() -- stops reading from the socket right after seeing "NBDMAGIC" (8 bytes). So that's a "broken pipe" situation (RST) for the sending party (the server). Ref: https://listman.redhat.com/archives/libguestfs/2022-March/028528.html Suggested-by: Richard W.M. Jones <rjones(a)redhat.com&gt; Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- Notes: v2: - new patch [Rich] p2v.h | 1 - conversion.c | 6 - nbd.c | 116 -------------------- 3 files changed, 123 deletions(-) diff --git a/p2v.h b/p2v.h index c55f64317dde..20c4ac7e506a 100644 --- a/p2v.h +++ b/p2v.h @@ -113,7 +113,6 @@ extern int scp_file (struct config *config, const char *target, const char *loca /* nbd.c */ extern void test_nbd_server (void); extern pid_t start_nbd_server (int *port, const char *device); -extern int wait_for_nbd_server_to_start (int port); const char *get_nbd_error (void); /* utils.c */ diff --git a/conversion.c b/conversion.c index 8bc4119fb47c..b9af47deda74 100644 --- a/conversion.c +++ b/conversion.c @@ -155,267 +155,261 @@ 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; 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) { 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; } diff --git a/nbd.c b/nbd.c index cd1b9ab6197b..39ce16d2b1d9 100644 --- a/nbd.c +++ b/nbd.c @@ -50,7 +50,6 @@ static bool nbd_exit_with_parent; 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_to_nbdkit (int dest_port); static char *nbd_error; @@ -279,202 +278,87 @@ 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_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. - */ -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 nbdkit to start"); - goto cleanup; - } - - 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 nbdkit 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 nbdkit 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 nbdkit to start: " - "'NBDMAGIC' was not received from nbdkit"); - 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 may involve multiple connections - to IPv4 and IPv6 for - * instance. - */ -static int -connect_to_nbdkit (int dest_port) -{ - struct addrinfo hints; - struct addrinfo *results, *rp; - char dest_port_str[16]; - int r, sockfd = -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; - - /* Connect. */ - if (connect (sockfd, rp->ai_addr, rp->ai_addrlen) == -1) { - set_nbd_error ("waiting for nbdkit to start: " - "connect to localhost/%s: %m", dest_port_str); - close (sockfd); - sockfd = -1; - continue; - } - - break; - } - - freeaddrinfo (results); - return sockfd; -} -- 2.19.1.3.g30247aa5d201

On 04/01/22 20:13, Richard W.M. Jones wrote: Commit range c1a4bd83a8bb..7388908f3892. Thank you! Laszlo