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

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