Laszlo Ersek
2022-Mar-31 07:22 UTC
[Libguestfs] [p2v PATCH 08/10] nbd.c: remove "enum nbd_server"
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
Richard W.M. Jones
2022-Mar-31 13:03 UTC
[Libguestfs] [p2v PATCH 08/10] nbd.c: remove "enum nbd_server"
For patches 4-8: Reviewed-by: Richard W.M. Jones <rjones at redhat.com> I would be tempted to squash a few of these together before pushing, but whatever you wish. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org