thr3ads.net - Libguestfs - [Libguestfs] [libnbd PATCH v5 3/4] generator: Add APIs to get/set the socket activation socket name [Mar 2023]

If this information is useful, please help other people find it:
Share via:

Laszlo Ersek

2023-Mar-25 11:39 UTC

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

From: "Richard W.M. Jones" <rjones at redhat.com>

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.

[Laszlo's notes:

- Originally posted by Rich at
 
<https://listman.redhat.com/archives/libguestfs/2023-January/030557.html>
  (Message-Id: <20230130225521.1771496-4-rjones at redhat.com>).

- I've renamed "sa_name" to "sact_name", due to
<signal.h> reserving
  symbols with the "sa_" prefix. This corresponds to earlier patches
in
  this series, such as 'socket activation: rename sa_(tmpdir|sockpath) to
  sact_(tmpdir|sockpath)' and 'ocaml: rename "sa_u" to
"saddr_u"'.

- Restricted "set_socket_activation_name" to the "Created"
state, per
  Eric's recommendation
 
<http://mid.mail-archive.com/n6lzl7ii5m4hu77n7rrzm3uih3tnx457e4y67rghki34b2ghh7
at 7vyini26mqko>.

- Documented that we are going to pass "unknown" if
"socket_name" is set
  to the empty string
 
<http://mid.mail-archive.com/oqwjnjvq4phqr76yum6zo5erfrm3tvmyewr5nxru3oxklobpgp
at 4plkku7opujw>.]

Signed-off-by: Laszlo Ersek <lersek at redhat.com>
Reviewed-by: Eric Blake <eblake at redhat.com>
---

Notes:
    v5:
    
    - restrict the set_socket_activation_name() API to the Created state
      [Eric]
    
    - Explain that not setting (or unsetting) the socket activation name
      results (per update to the next patch) in the child NBD server seeing
      "unknown" [Eric, Laszlo]
    
    - update the commit message accordingly (as the commit message is still
      based on Rich's original); also move the Message-Id tag (identifying
      Rich's original patch) next to the mailing list archive URL
    
    - drop Rich's R-b due to the above changes
    
    - pick up Eric's R-b
    
    v4:
    
    - pick up Rich's R-b

 lib/internal.h   |  1 +
 generator/API.ml | 50 +++++++++++++++++
 lib/handle.c     | 56 ++++++++++++++++++++
 3 files changed, 107 insertions(+)

diff --git a/lib/internal.h b/lib/internal.h
index 35cb5e8994ee..2de8e4e5e043 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 *sact_name;              /* Socket activation name, can be NULL. */
 
   /* TLS settings. */
   int tls;                      /* 0 = disable, 1 = enable, 2 = require */
diff --git a/generator/API.ml b/generator/API.ml
index 91e57a4c7c4f..bbb0b8a6e1ee 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -2036,15 +2036,63 @@   "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;
+    permitted_states = [ Created ];
+    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 the name C<unknown> will be seen by 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 +3892,8 @@   "get_uri", {
   "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 8468b964240b..0f11bee56221 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
@@ -159,6 +160,7 @@ nbd_close (struct nbd_handle *h)
     waitpid (h->pid, NULL, 0);
 
   free (h->export_name);
+  free (h->sact_name);
   free (h->tls_certificates);
   free (h->tls_username);
   free (h->tls_psk_file);
@@ -197,6 +199,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->sact_name);
+    h->sact_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->sact_name);
+  h->sact_name = new_name;
+  return 0;
+}
+
+char *
+nbd_unlocked_get_socket_activation_name (struct nbd_handle *h)
+{
+  char *copy = strdup (h->sact_name ? h->sact_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)
 {

Eric Blake

2023-Mar-28 02:06 UTC

head link

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

On Sat, Mar 25, 2023 at 12:39:28PM +0100, Laszlo Ersek
wrote:> From: "Richard W.M. Jones" <rjones at redhat.com>
> 
> 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.
> 
> [Laszlo's notes:
> 
> - Originally posted by Rich at
>  
<https://listman.redhat.com/archives/libguestfs/2023-January/030557.html>
>   (Message-Id: <20230130225521.1771496-4-rjones at redhat.com>).
> 
> - I've renamed "sa_name" to "sact_name", due to
<signal.h> reserving
>   symbols with the "sa_" prefix. This corresponds to earlier
patches in
>   this series, such as 'socket activation: rename sa_(tmpdir|sockpath)
to
>   sact_(tmpdir|sockpath)' and 'ocaml: rename "sa_u" to
"saddr_u"'.
> 
> - Restricted "set_socket_activation_name" to the
"Created" state, per
>   Eric's recommendation
>  
<http://mid.mail-archive.com/n6lzl7ii5m4hu77n7rrzm3uih3tnx457e4y67rghki34b2ghh7
at 7vyini26mqko>.
> 
> - Documented that we are going to pass "unknown" if
"socket_name" is set
>   to the empty string
>  
<http://mid.mail-archive.com/oqwjnjvq4phqr76yum6zo5erfrm3tvmyewr5nxru3oxklobpgp
at 4plkku7opujw>.]
> 
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> Reviewed-by: Eric Blake <eblake at redhat.com>
> ---
> 
>  
> +  "set_socket_activation_name", {
> +    default_call with
> +    args = [ String "socket_name" ]; ret = RErr;
> +    permitted_states = [ Created ];
> +    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 the name C<unknown> will be seen by the
server.";
Accurate,
> +    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.";
slightly misleading. Maybe: 'By default this will return the empty
string meaning that a server will see the name C<unknown>'.

Other than that, this series looks good to go from my end.

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

Possibly Parallel Threads

Search for more possibly parallel threads

Libguestfs - Mar 2023 - [libnbd PATCH v5 3/4] generator: Add APIs to get/set the socket activation socket name

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

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

Possibly Parallel Threads