[Libguestfs] [p2v PATCH 08/10] nbd.c: remove "enum nbd_server"

Laszlo Ersek
2022-03-31T07:22:09Z

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 at redhat.com>
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 main.c       |   2 +-
 nbd.c        | 110 ++++++--------------
 p2v.h        |   2 +-
 virt-p2v.pod |   6 +-
 4 files changed, 34 insertions(+), 86 deletions(-)

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