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 <rjo...@redhat.com> > Signed-off-by: Laszlo Ersek <ler...@redhat.com>
Reviewed-by: Richard W.M. Jones <rjo...@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@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 _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs