[Libguestfs] [p2v PATCH 06/10] remove "--nbd" option parsing

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

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 at redhat.com>
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 main.c                  |  5 --
 nbd.c                   | 64 +++-----------------
 p2v.h                   |  1 -
 test-virt-p2v-nbdkit.sh |  5 +-
 virt-p2v.pod            | 14 -----
 5 files changed, 10 insertions(+), 79 deletions(-)

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/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/test-virt-p2v-nbdkit.sh b/test-virt-p2v-nbdkit.sh
index 99bb8d9fe293..68696f06605f 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

@@ -46,8 +46,7 @@ export PATH=$d:$PATH
 # The Linux kernel command line.
 cmdline="p2v.server=localhost p2v.name=fedora p2v.disks=$f1,$f2
p2v.o=local p2v.os=$(pwd)/$d p2v.network=em1:wired,other p2v.post="

-# Only use nbdkit.
-$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