[Libguestfs] [PATCH libnbd v2 3/4] generator: Add APIs to get/set the socket activation socket name

[Eric Blake]

On Mon, Jan 30, 2023 at 10:55:20PM +0000, 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.

This creates an implicit client-side stateful API: to pass socket
names, you must call two APIs in the correct sequence:

nbd_set_socket_activation_name (h, "foo");
nbd_connect_systemd_socket_activation (h, argv);

I can live with that design, but I recall Laszlo questioning in the
past if we always need to force this stateful design onto clients, or
if it would be easier to instead add a alternative API that takes the
socket name as an additional parameter, in one shot:

nbd_connect_systemd_named_socket_activation (h, "foo", argv);

>  
> +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");

I don't mind keeping us strict to start with and loosening it later if
someone demonstrates why it is needed.  But systemd documents a larger
set of possible names:

https://www.freedesktop.org/software/systemd/man/sd_pid_notify_with_fds.html

FDNAME=…

    When used in combination with FDSTORE=1, specifies a name for the
    submitted file descriptors. When used with FDSTOREREMOVE=1,
    specifies the name for the file descriptors to remove. This name
    is passed to the service during activation, and may be queried
    using sd_listen_fds_with_names(3). File descriptors submitted
    without this field set, will implicitly get the name "stored"
    assigned. Note that, if multiple file descriptors are submitted at
    once, the specified name will be assigned to all of them. In order
    to assign different names to submitted file descriptors, submit
    them in separate invocations of sd_pid_notify_with_fds(). The name
    may consist of arbitrary ASCII characters except control
    characters or ":". It may not be longer than 255 characters. If a
    submitted name does not follow these restrictions, it is ignored.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org