5:27 a.m.
According to the first commit message in this series, remove support for
parsing "--nbd=nbdkit-no-sa".
Systemd socket activation was added to nbdkit in nbdkit commit
7ff39d028c63 ("Add support for socket activation.", 2017-01-31); first
released in nbdkit v1.1.13 (Aug 2017).
Note that there is a non-intuitive change in this patch: in the
test_nbd_servers() function, we remove the grepping for the "LISTEN_PID"
string in the "nbdkit" binary. In other words, we stop verifying whether
"nbdkit" indeed supports socket activation, we just assume it.
The primary reason for this is that this check never actually worked when
the nbdkit binary was exposed from the root directory of an nbdkit build
tree! That binary is actually built from "wrapper.c", and it's only the
"server/nbdkit" binary that matches "LISTEN_PID". However, as
"wrapper.c"
explains, "server/nbdkit" would use the system-wide nbdkit plugins, not
the just-built ones, therefore only the wrapper "nbdkit" executable can be
used -- and so the "LISTEN_PID" grep command has always been unable to
deal with that situation. Unless we remove that check too, with the rest
of this patch, virt-p2v refuses to start up -- as no usable nbd server is
found.
This patch permits further simplification, which will be done subsequently
in this series.
Ref: https://listman.redhat.com/archives/libguestfs/2022-March/028475.html
Suggested-by: Richard W.M. Jones <rjones(a)redhat.com>
Signed-off-by: Laszlo Ersek <lersek(a)redhat.com>
Reviewed-by: Richard W.M. Jones <rjones(a)redhat.com>
---
Notes:
v2:
- in the commit message, highlight the nbdkit commit that introduced
systemd socket activation [Rich]
- pick up Rich's R-b
nbd.c | 42 +-------------------
test-virt-p2v-nbdkit.sh | 2 +-
virt-p2v.pod | 18 +++------
3 files changed, 8 insertions(+), 54 deletions(-)
diff --git a/nbd.c b/nbd.c
index 46f6304d2c51..edc3ab51d889 100644
--- a/nbd.c
+++ b/nbd.c
@@ -50,8 +50,7 @@ static int nbd_local_port;
/* List of servers specified by the --nbd option. */
enum nbd_server {
/* 0 is reserved for "end of list" */
NBDKIT = 1,
- NBDKIT_NO_SA = 2,
};
static enum nbd_server *cmdline_servers = NULL;
@@ -59,31 +58,29 @@ static const char *
nbd_server_string (enum nbd_server s)
{
const char *ret = NULL;
switch (s) {
case NBDKIT: ret = "nbdkit"; break;
- case NBDKIT_NO_SA: ret = "nbdkit-no-sa"; 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).
*/
static const enum nbd_server standard_servers[] =
- { NBDKIT, NBDKIT_NO_SA, 0 };
+ { 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, 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);
static int bind_tcpip_socket (const char *ipaddr, const char *port, int **fds, size_t
*nr_fds);
static int connect_with_source_port (const char *hostname, int dest_port, int
source_port);
@@ -126,40 +123,38 @@ 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 if (STREQ (strs[i], "nbdkit-no-sa"))
- cmdline_servers[i] = NBDKIT_NO_SA;
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.
*
* Set the C<use_server> global accordingly.
*/
@@ -167,88 +162,75 @@ 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;
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
- " && grep -sq LISTEN_PID `which nbdkit`"
- );
- if (r == 0) {
- use_server = servers[i];
- goto finish;
- }
- break;
-
- case NBDKIT_NO_SA:
- 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"),
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.
*/
@@ -256,35 +238,29 @@ pid_t
start_nbd_server (const char **ipaddr, 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 */
*ipaddr = "localhost";
*port = open_listening_socket (*ipaddr, &fds, &nr_fds);
if (*port == -1) return -1;
pid = start_nbdkit (device, *ipaddr, *port, fds, nr_fds);
for (i = 0; i < nr_fds; ++i)
close (fds[i]);
free (fds);
return pid;
-
- case NBDKIT_NO_SA: /* nbdkit without socket activation */
- *ipaddr = "localhost";
- *port = get_local_port ();
- if (*port == -1) return -1;
- return start_nbdkit (device, *ipaddr, *port, NULL, 0);
}
abort ();
}
#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.
*/
@@ -325,89 +301,73 @@ static pid_t
start_nbdkit (const char *device,
const char *ipaddr, int port, int *fds, size_t nr_fds)
{
pid_t pid;
char port_str[64];
CLEANUP_FREE char *file_str = NULL;
#if DEBUG_STDERR
fprintf (stderr, "starting nbdkit for %s on %s:%d%s\n",
device, ipaddr, port,
fds == NULL ? "" : " using socket activation");
#endif
snprintf (port_str, sizeof port_str, "%d", port);
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);
}
if (fds == NULL) { /* without socket activation */
execlp ("nbdkit",
"nbdkit",
"-r", /* readonly (vital!) */
"-p", port_str, /* listening port */
"-i", ipaddr, /* listen only on loopback interface */
"-f", /* don't fork */
"file", /* file plugin */
file_str, /* a device like file=/dev/sda */
NULL);
perror ("nbdkit");
_exit (EXIT_FAILURE);
}
else { /* socket activation */
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 that does not
- * support socket activation. We have to pass the '-p' option to
- * the NBD server, but there's no good way to choose a free port,
- * so we have to just guess.
- *
- * Returns the port number on success or C<-1> on error.
- */
-static int
-get_local_port (void)
-{
- int port = nbd_local_port;
- nbd_local_port++;
- return port;
-}
-
/**
* 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.
*
* 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.
*/
diff --git a/test-virt-p2v-nbdkit.sh b/test-virt-p2v-nbdkit.sh
index bba4bda4ab87..0b020cfe4bed 100755
--- a/test-virt-p2v-nbdkit.sh
+++ b/test-virt-p2v-nbdkit.sh
@@ -48,7 +48,7 @@ export PATH=$d:$PATH
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,nbdkit-no-sa
+$VG virt-p2v --cmdline="$cmdline" --nbd=nbdkit
# 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 eb220cbf1cee..e6cbb1514edc 100644
--- a/virt-p2v.pod
+++ b/virt-p2v.pod
@@ -509,24 +509,17 @@ 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,nbdkit-no-sa>
+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)>).
-
-=item B<nbdkit-no-sa>
-
-Use nbdkit, but disable socket activation
+Use nbdkit with the file plugin (see: L<nbdkit-file-plugin(1)>) and
+socket activation.
=back
-The C<nbdkit-no-sa> variant allows virt-p2v to fall back to older
-versions of nbdkit which did not support L<socket
-activation|http://0pointer.de/blog/projects/socket-activation.html>.
-
=item B<--test-disk=/PATH/TO/DISK.IMG>
For testing or debugging purposes, replace F</dev/sda> with a local
@@ -670,8 +663,9 @@ Before conversion actually begins, virt-p2v then makes one or more
further ssh connections to the server for data transfer.
The transfer protocol used currently is NBD (Network Block Device),
-which is proxied over ssh. The NBD server is L<nbdkit(1)>; socket
-activation can be controlled using the I<--nbd> command line option.
+which is proxied over ssh. The NBD server is L<nbdkit(1)>, with
+L<nbdkit-file-plugin(1)> and L<socket
+activation|http://0pointer.de/blog/projects/socket-activation.html>.
There is one ssh connection per physical hard disk on the source
machine (the common case — a single hard disk — is shown below):
--
2.19.1.3.g30247aa5d201