remove qemu-nbd support

On 03/24/22 10:13, Richard W.M. Jones wrote:
> virt-p2v needs an NBD server running on the p2v machine to export the
> disks.  Originally we used qemu-nbd since that was the only choice.
> In 2017, I added support for nbdkit as an alternative to qemu-nbd.
>
> So now we're in the situation where either server can be used (see
> virt-p2v.git/nbd.c).  For added complexity we also support both
> servers in either systemd socket activation (SA) mode or "no-SA" mode,
> so that's 4 combinations.
>
> This is silly, we should support only one NBD server, and since
> systemd socket activation is well-supported and more flexible, we
> should just use it.
>
> So the question is *which* NBD server to support.  That's not so much
> a technical matter since both servers can easily serve a local block
> device (always raw format).  However I do think that nbdkit might
> genuinely be the better choice here:
>
>  - qemu-nbd links to the whole qemu block layer, nbdkit can be shipped
>    with just the plugin we need, so it should be smaller with less
>    code surface
>
>  - nbdkit-file-plugin has a better method of not trashing the host
>    page cache
>
>  - could use nbdkit --exit-with-parent feature (which we don't at the
>    moment)
>
>  - nbdkit is widely available in distros these days
>
> Also that file uses AI_ADDRCONFIG so I guess it has problems with IPv6.

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 <[email protected]>
Signed-off-by: Laszlo Ersek <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Richard W.M. Jones <[email protected]>

Author: Laszlo Ersek

Makefile.am

@@ -31,7 +31,6 @@ EXTRA_DIST = \
 	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 \
@@ -339,7 +338,6 @@ TESTS = \
 	test-virt-p2v-docs.sh
 
 LIBGUESTFS_TESTS = \
-	test-virt-p2v.sh \
 	test-virt-p2v-nbdkit.sh
 
 if HAVE_LIBGUESTFS

contrib/aux-scripts/do-build.sh

@@ -59,7 +59,6 @@ case $osversion in
         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.
@@ -122,7 +121,8 @@ case $osversion in
             -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.
@@ -137,7 +137,8 @@ case $osversion in
             -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 \

contrib/patches/0002-RHEL-5-ONLY-QEMU-NBD-1.4-HAS-NO-f-OPTION.patch

@@ -29,7 +29,8 @@ ifelse(REDHAT,1,
 
   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
@@ -66,7 +67,7 @@ ifelse(DEBIAN,1,
   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
@@ -86,7 +87,7 @@ ifelse(ARCHLINUX,1,
   gtk`'GTK_VERSION
   dbus
   openssh
-  qemu
+  nbdkit
   which
   vim-tiny
   open-iscsi
@@ -107,7 +108,8 @@ ifelse(SUSE,1,
   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
@@ -134,7 +136,8 @@ ifelse(OPENMANDRIVA,1,
 
   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

dependencies.m4

@@ -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
 

docs/p2v-building.pod

@@ -99,7 +99,7 @@ usage (int status)
               " --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"

main.c

@@ -17,7 +17,7 @@
 
 /**
  * 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>
@@ -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;
 
@@ -63,8 +61,6 @@ 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;
   }
@@ -79,14 +75,13 @@ nbd_server_string (enum nbd_server s)
  * 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);
@@ -150,11 +145,7 @@ set_nbd_option (const char *opt)
     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;
@@ -205,31 +196,6 @@ test_nbd_servers (void)
 #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
@@ -294,27 +260,6 @@ start_nbd_server (const char **ipaddr, int *port, const char *device)
   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);
@@ -366,77 +311,6 @@ socket_activation (int *fds, size_t nr_fds)
   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)>.

nbd.c

@@ -15,7 +15,7 @@
 # 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.

test-virt-p2v-nbdkit.sh

@@ -14,7 +14,7 @@
 # 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:' \

test-virt-p2v-scp.sh

@@ -14,7 +14,7 @@
 # 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:' \
@@ -37,7 +37,7 @@ while true ; do
 
         # 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}')"