Libguestfs - Mar 2023 - [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

On 3/28/23 04:06, Eric Blake wrote:
> 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>'.
Ah, right. Sorry about missing this, I've been forwarding/tweaking this
somewhat opaquely from Rich.
> 
> Other than that, this series looks good to go from my end.
> 
Thanks, I'll fix this up when I push the series!

Laszlo