[Libguestfs] [p2v PATCH 03/10] nbd.c: simplify start_nbdkit()
Richard W.M. Jones
rjones at redhat.com
Thu Mar 31 12:57:25 UTC 2022
On Thu, Mar 31, 2022 at 09:22:04AM +0200, Laszlo Ersek wrote:
> The previous patch guarantees that start_nbdkit() is only called with a
> non-NULL "fds" parameter (and, consequently, that start_nbdkit() does not
> use "ipaddr" and "port" for anything beyond logging).
>
> Remove the branch in start_nbdkit() that starts nbdkit without socket
> activation, and trim the start_nbdkit() prototype too.
>
> This patch is best viewed with "git show -b" due to the unindentation in
> it.
>
> 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>
Reviewed-by: Richard W.M. Jones <rjones at redhat.com>
Rich.
> nbd.c | 54 ++++++--------------
> 1 file changed, 16 insertions(+), 38 deletions(-)
>
> diff --git a/nbd.c b/nbd.c
> index edc3ab51d889..cda962d03bdd 100644
> --- a/nbd.c
> +++ b/nbd.c
> @@ -80,7 +80,7 @@ static const enum nbd_server standard_servers[] =
> */
> 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 pid_t start_nbdkit (const char *device, int *fds, size_t nr_fds);
> 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);
> @@ -238,29 +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);
> + pid = start_nbdkit (device, fds, nr_fds);
> for (i = 0; i < nr_fds; ++i)
> close (fds[i]);
> free (fds);
> return pid;
> }
>
> 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.
> */
> @@ -268,106 +268,84 @@ static inline void
> socket_activation (int *fds, size_t nr_fds)
> {
> size_t i;
> char nr_fds_str[16];
> char pid_str[16];
>
> if (fds == NULL) return;
>
> for (i = 0; i < nr_fds; ++i) {
> int fd = FIRST_SOCKET_ACTIVATION_FD + i;
> if (fds[i] != fd) {
> dup2 (fds[i], fd);
> close (fds[i]);
> }
> }
>
> snprintf (nr_fds_str, sizeof nr_fds_str, "%zu", nr_fds);
> setenv ("LISTEN_FDS", nr_fds_str, 1);
> snprintf (pid_str, sizeof pid_str, "%d", (int) getpid ());
> setenv ("LISTEN_PID", pid_str, 1);
> }
>
> /**
> * Start a local L<nbdkit(1)> process using the
> * L<nbdkit-file-plugin(1)>.
> *
> - * If we are using socket activation, C<fds> and C<nr_fds> will
> - * contain the locally pre-opened file descriptors for this.
> - * Otherwise if C<fds == NULL> we pass the port number.
> + * C<fds> and C<nr_fds> will contain the locally pre-opened file descriptors
> + * for this.
> *
> * Returns the process ID (E<gt> 0) or C<0> if there is an error.
> */
> static pid_t
> -start_nbdkit (const char *device,
> - const char *ipaddr, int port, int *fds, size_t nr_fds)
> +start_nbdkit (const char *device, 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");
> + fprintf (stderr, "starting nbdkit for %s using socket activation\n", device);
> #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);
> + 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);
> - }
> + 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.
> *
> * 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.
> */
> --
> 2.19.1.3.g30247aa5d201
>
>
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://listman.redhat.com/mailman/listinfo/libguestfs
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
More information about the Libguestfs
mailing list