Previously there were two places where similiar-ish logic was used to
decide if we are going to serve over a socket activation, -s, Unix
socket, AF_VSOCK or TCP/IP.  Let's abstract that into a service_mode.

One place where we did this was when calculating the $uri variable for
--run.  This change adjusts and fixes this calculation (revealed as I
made the above change).  In particular if --port was not set then the
$uri would contain fairly bogus values in some cases:

  $ nbdkit --vsock null 1M --run 'echo $uri ; nbdinfo $uri'
  nbd
  nbdinfo: nbd_connect_uri: NBD URI does not have a scheme: valid NBD URIs 
should start with a scheme like nbd://, nbds:// or nbd+unix://: Invalid argument

(note uri='nbd')

After this commit:

  $ nbdkit --vsock null 1M --run 'echo $uri ; nbdinfo $uri'
  nbd+vsock://1
  protocol: newstyle-fixed without TLS, using structured packets
  export="":
        export-size: 1048576 (1M)
        content: data
        uri: nbd+vsock://1:10809/
  ...
---
 server/internal.h | 11 +++++++
 server/captive.c  | 56 ++++++++++++++++++++---------------
 server/main.c     | 75 ++++++++++++++++++++++++++++++-----------------
 3 files changed, 91 insertions(+), 51 deletions(-)

diff --git a/server/internal.h b/server/internal.h
index fcfbf0573..0652f8a86 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -108,6 +108,16 @@ enum log_to {
   LOG_TO_NULL,           /* --log=null forced on the command line */
 };
 
+enum service_mode {
+  /* These two modes cannot form an NBD URI: */
+  SERVICE_MODE_SOCKET_ACTIVATION, /* socket activation. */
+  SERVICE_MODE_LISTEN_STDIN,      /* -s */
+
+  SERVICE_MODE_UNIXSOCKET,        /* -U */
+  SERVICE_MODE_VSOCK,             /* --vsock */
+  SERVICE_MODE_TCPIP,             /* --port */
+};
+
 extern int tcpip_sock_af;
 extern struct debug_flag *debug_flags;
 extern const char *export_name;
@@ -131,6 +141,7 @@ extern char *unixsocket;
 extern const char *user, *group;
 extern bool verbose;
 extern bool vsock;
+extern enum service_mode service_mode;
 extern bool configured;
 extern int saved_stdin;
 extern int saved_stdout;
diff --git a/server/captive.c b/server/captive.c
index 2361bb60b..31fd949e5 100644
--- a/server/captive.c
+++ b/server/captive.c
@@ -78,36 +78,44 @@ run_command (void)
 
   /* Construct $uri. */
   fprintf (fp, "uri=");
-  if (tls == 2)                 /* --tls=require */
-    fprintf (fp, "nbds");
-  else
-    fprintf (fp, "nbd");
-  if (port) {
-    if (!vsock) {
-      fprintf (fp, "://localhost:");
-      shell_quote (port, fp);
-      if (strcmp (export_name, "") != 0) {
-        putc ('/', fp);
-        uri_quote (export_name, fp);
-      }
-    }
-    else {
-      fprintf (fp, "+vsock://1:"); /* 1 = VMADDR_CID_LOCAL */
-      shell_quote (port, fp);
-      if (strcmp (export_name, "") != 0) {
-        putc ('/', fp);
-        uri_quote (export_name, fp);
-      }
-    }
-  }
-  else if (unixsocket) {
-    fprintf (fp, "+unix://");
+  switch (service_mode) {
+  case SERVICE_MODE_SOCKET_ACTIVATION:
+  case SERVICE_MODE_LISTEN_STDIN:
+    break;                      /* can't form a URI, leave it blank */
+  case SERVICE_MODE_UNIXSOCKET:
+    fprintf (fp, "nbd%s+unix://", tls == 2 ? "s" : "");
     if (strcmp (export_name, "") != 0) {
       putc ('/', fp);
       uri_quote (export_name, fp);
     }
     fprintf (fp, "\\?socket=");
     uri_quote (unixsocket, fp);
+    break;
+  case SERVICE_MODE_VSOCK:
+    /* 1 = VMADDR_CID_LOCAL */
+    fprintf (fp, "nbd%s+vsock://1", tls == 2 ? "s" : "");
+    if (port) {
+      putc (':', fp);
+      shell_quote (port, fp);
+    }
+    if (strcmp (export_name, "") != 0) {
+      putc ('/', fp);
+      uri_quote (export_name, fp);
+    }
+    break;
+  case SERVICE_MODE_TCPIP:
+    fprintf (fp, "nbd%s://localhost", tls == 2 ? "s" : "");
+    if (port) {
+      putc (':', fp);
+      shell_quote (port, fp);
+    }
+    if (strcmp (export_name, "") != 0) {
+      putc ('/', fp);
+      uri_quote (export_name, fp);
+    }
+    break;
+  default:
+    abort ();
   }
   putc ('\n', fp);
 
diff --git a/server/main.c b/server/main.c
index 528a2dfea..2a332bfdd 100644
--- a/server/main.c
+++ b/server/main.c
@@ -117,6 +117,7 @@ const char *user, *group;       /* -u & -g */
 bool verbose;                   /* -v */
 bool vsock;                     /* --vsock */
 unsigned int socket_activation; /* $LISTEN_FDS and $LISTEN_PID set */
+enum service_mode service_mode; /* serving over TCP, Unix, etc */
 bool configured;                /* .config_complete done */
 int saved_stdin = -1;           /* dup'd stdin during -s/--run */
 int saved_stdout = -1;          /* dup'd stdout during -s/--run */
@@ -617,6 +618,18 @@ main (int argc, char *argv[])
     exit (EXIT_FAILURE);
   }
 
+  /* By the point we have enough information to calculate the service mode. */
+  if (socket_activation)
+    service_mode = SERVICE_MODE_SOCKET_ACTIVATION;
+  else if (listen_stdin)
+    service_mode = SERVICE_MODE_LISTEN_STDIN;
+  else if (unixsocket)
+    service_mode = SERVICE_MODE_UNIXSOCKET;
+  else if (vsock)
+    service_mode = SERVICE_MODE_VSOCK;
+  else
+    service_mode = SERVICE_MODE_TCPIP;
+
   /* The remaining command line arguments are the plugin name and
    * parameters.  If --help, --version or --dump-plugin were specified
    * then we open the plugin so that we can display the per-plugin
@@ -970,15 +983,16 @@ start_serving (void)
 #endif
   }
 
-  /* Socket activation: the ‘socket_activation’ variable (> 0) is the
-   * number of file descriptors from FIRST_SOCKET_ACTIVATION_FD to
-   * FIRST_SOCKET_ACTIVATION_FD+socket_activation-1.
-   */
-  if (socket_activation) {
-      if (sockets_reserve (&socks, socket_activation) == -1) {
-        perror ("realloc");
-        exit (EXIT_FAILURE);
-      }
+  switch (service_mode) {
+  case SERVICE_MODE_SOCKET_ACTIVATION:
+    /* Socket activation: the ‘socket_activation’ variable (> 0) is
+     * the number of file descriptors from FIRST_SOCKET_ACTIVATION_FD
+     * to FIRST_SOCKET_ACTIVATION_FD+socket_activation-1.
+     */
+    if (sockets_reserve (&socks, socket_activation) == -1) {
+      perror ("realloc");
+      exit (EXIT_FAILURE);
+    }
     for (i = 0; i < socket_activation; ++i) {
       int s = FIRST_SOCKET_ACTIVATION_FD + i, r;
       /* This can't fail because of the reservation above. */
@@ -990,35 +1004,42 @@ start_serving (void)
     write_pidfile ();
     top->after_fork (top);
     accept_incoming_connections (&socks);
-    return;
-  }
+    break;
 
-  /* Handling a single connection on stdin/stdout. */
-  if (listen_stdin) {
+  case SERVICE_MODE_LISTEN_STDIN:
+    /* Handling a single connection on stdin/stdout. */
     change_user ();
     write_pidfile ();
     top->after_fork (top);
     threadlocal_new_server_thread ();
     handle_single_connection (saved_stdin, saved_stdout);
-    return;
-  }
+    break;
 
-  /* Handling multiple connections on TCP/IP, Unix domain socket or
-   * AF_VSOCK.
-   */
-  if (unixsocket)
+  case SERVICE_MODE_UNIXSOCKET:
     bind_unix_socket (&socks);
-  else if (vsock)
+    goto serve_socks;
+
+  case SERVICE_MODE_VSOCK:
     bind_vsock (&socks);
-  else
+    goto serve_socks;
+
+  case SERVICE_MODE_TCPIP:
     bind_tcpip_socket (&socks);
+  serve_socks:
+    /* Common code for handling multiple connections on TCP/IP, Unix
+     * domain socket or AF_VSOCK.
+     */
+    run_command ();
+    change_user ();
+    fork_into_background ();
+    write_pidfile ();
+    top->after_fork (top);
+    accept_incoming_connections (&socks);
+    break;
 
-  run_command ();
-  change_user ();
-  fork_into_background ();
-  write_pidfile ();
-  top->after_fork (top);
-  accept_incoming_connections (&socks);
+  default:
+    abort ();
+  }
 }
 
 static void
-- 
2.41.0

_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Reply via email to