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