On 1/30/23 23:55, Richard W.M. Jones wrote:
> To allow us to name the socket passed down to the NBD server when
> calling nbd_connect_systemd_socket_activation(3), we need to add the
> field to the handle and add access functions.
> ---
> generator/API.ml | 49 ++++++++++++++++++++++++++++++++++++++++++
> lib/handle.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++
> lib/internal.h | 1 +
> 3 files changed, 106 insertions(+)
>
> diff --git a/generator/API.ml b/generator/API.ml
> index 25a612a2e8..08fc80960b 100644
> --- a/generator/API.ml
> +++ b/generator/API.ml
> @@ -2036,15 +2036,62 @@ "connect_systemd_socket_activation", {
>
> When the NBD handle is closed the server subprocess
> is killed.
> +
> +=head3 Socket name
> +
> +The socket activation protocol lets you optionally give
> +the socket a name. If used, the name is passed to the
> +NBD server using the C<LISTEN_FDNAMES> environment
> +variable. To provide a socket name, call
> +L<nbd_set_socket_activation_name(3)> before calling
> +the connect function.
> " ^ blocking_connect_call_description;
> see_also = [Link "aio_connect_systemd_socket_activation";
> Link "connect_command"; Link "kill_subprocess";
> Link "set_opt_mode";
> + Link "set_socket_activation_name";
> + Link "get_socket_activation_name";
> ExternalLink ("qemu-nbd", 1);
> URLLink
> "http://0pointer.de/blog/projects/socket-activation.html";];
> example = Some "examples/open-qcow2.c";
> };
>
> + "set_socket_activation_name", {
> + default_call with
> + args = [ String "socket_name" ]; ret = RErr;
> + shortdesc = "set the socket activation name";
> + longdesc = "\
> +When running an NBD server using
> +L<nbd_connect_systemd_socket_activation(3)> you can optionally
> +name the socket. Call this function before connecting to the
> +server.
> +
> +Some servers such as L<qemu-storage-daemon(1)>
> +can use this information to associate the socket with a name
> +used on the command line, but most servers will ignore it.
> +The name is passed through the C<LISTEN_FDNAMES> environment
> +variable.
> +
> +The parameter C<socket_name> can be a short alphanumeric string.
> +If it is set to the empty string (also the default when the handle
> +is created) then no name is passed to the server.";
> + see_also = [Link "connect_systemd_socket_activation";
> + Link "get_socket_activation_name"];
> + };
> +
> + "get_socket_activation_name", {
> + default_call with
> + args = []; ret = RString;
> + shortdesc = "get the socket activation name";
> + longdesc = "\
> +Return the socket name used when you call
> +L<nbd_connect_systemd_socket_activation(3)> on the same
> +handle. By default this will return the empty string
> +meaning that no name is passed to the server.";
> + see_also = [Link "connect_systemd_socket_activation";
> + Link "set_socket_activation_name"];
> + };
> +
> "is_read_only", {
> default_call with
> args = []; ret = RBool;
> @@ -3844,6 +3891,8 @@ let first_version =
> "aio_opt_structured_reply", (1, 16);
> "opt_starttls", (1, 16);
> "aio_opt_starttls", (1, 16);
> + "set_socket_activation_name", (1, 16);
> + "get_socket_activation_name", (1, 16);
>
> (* These calls are proposed for a future version of libnbd, but
> * have not been added to any released version so far.
> diff --git a/lib/handle.c b/lib/handle.c
> index 4a186f8fa9..96c8b1f2b1 100644
> --- a/lib/handle.c
> +++ b/lib/handle.c
> @@ -28,6 +28,7 @@
> #include <sys/types.h>
> #include <sys/wait.h>
>
> +#include "ascii-ctype.h"
> #include "internal.h"
>
> static void
> @@ -161,6 +162,7 @@ nbd_close (struct nbd_handle *h)
> waitpid (h->pid, NULL, 0);
>
> free (h->export_name);
> + free (h->sa_name);
> free (h->tls_certificates);
> free (h->tls_username);
> free (h->tls_psk_file);
> @@ -200,6 +202,60 @@ nbd_unlocked_get_handle_name (struct nbd_handle *h)
> return copy;
> }
>
> +int
> +nbd_unlocked_set_socket_activation_name (struct nbd_handle *h,
> + const char *name)
> +{
> + size_t i, len;
> + char *new_name;
> +
> + len = strlen (name);
> +
> + /* Setting it to empty string stores NULL in the handle. */
> + if (len == 0) {
> + free (h->sa_name);
> + h->sa_name = NULL;
> + return 0;
> + }
> +
> + /* Check the proposed name is short and alphanumeric. */
> + if (len > 32) {
> + set_error (ENAMETOOLONG, "socket activation name should be "
> + "<= 32 characters");
> + return -1;
> + }
> + for (i = 0; i < len; ++i) {
> + if (! ascii_isalnum (name[i])) {
> + set_error (EINVAL, "socket activation name should contain "
> + "only alphanumeric ASCII characters");
> + return -1;
> + }
> + }
> +
> + new_name = strdup (name);
> + if (!new_name) {
> + set_error (errno, "strdup");
> + return -1;
> + }
> +
> + free (h->sa_name);
> + h->sa_name = new_name;
> + return 0;
> +}
> +
> +char *
> +nbd_unlocked_get_socket_activation_name (struct nbd_handle *h)
> +{
> + char *copy = strdup (h->sa_name ? h->sa_name : "");
> +
> + if (!copy) {
> + set_error (errno, "strdup");
> + return NULL;
> + }
> +
> + return copy;
> +}
> +
> uintptr_t
> nbd_unlocked_set_private_data (struct nbd_handle *h, uintptr_t data)
> {
> diff --git a/lib/internal.h b/lib/internal.h
> index bbbd26393f..19d7f0af60 100644
> --- a/lib/internal.h
> +++ b/lib/internal.h
> @@ -101,6 +101,7 @@ struct nbd_handle {
> _Atomic uintptr_t private_data;
>
> char *export_name; /* Export name, never NULL. */
> + char *sa_name; /* Socket activation name, can be NULL. */
>
> /* TLS settings. */
> int tls; /* 0 = disable, 1 = enable, 2 = require */
"lib/handle.c" includes <signal.h>, and <signal.h> reserves symbols
starting with the prefix "sa_":
https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_02_02
While the fields of unions and structures live in a separate namespace,
that's no protection from macros.
Otherwise the patch looks OK to me.
(I'm noticing we already have fields called "sa_tmpdir" and
"sa_sockpath"; that's not ideal, although it may not matter in practice.)
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
Laszlo
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
