On 1/31/23 17:38, Eric Blake wrote:
> On Mon, Jan 30, 2023 at 10:55:21PM +0000, Richard W.M. Jones wrote:
>> When the user calls nbd_set_socket_activation_name before calling
>> nbd_connect_system_socket_activation, pass the name down to the server
>> through LISTEN_FDNAMES. This has no effect unless the new API has
>> been called to set the socket name to a non-empty string.
>> ---
>> generator/states-connect-socket-activation.c | 35 +++++++++++++++-----
>> 1 file changed, 26 insertions(+), 9 deletions(-)
>>
>
>> + *
>> + * env[2] (if used) is "LISTEN_FDNAMES=" + h->sa_name
>
>> @@ -61,11 +69,20 @@ prepare_socket_activation_environment (string_vector
>> *env)
>> if (p == NULL)
>> goto err;
>> string_vector_append (env, p);
>> + if (using_name) {
>> + if (asprintf (&p, "LISTEN_FDNAMES=%s", h->sa_name) == -1)
>> + goto err;
>> + string_vector_append (env, p);
>> + }
>>
>> - /* Append the current environment, but remove LISTEN_PID, LISTEN_FDS. */
>> + /* Append the current environment, but remove the special
>> + * environment variables.
>> + */
>> for (i = 0; environ[i] != NULL; ++i) {
>> if (strncmp (environ[i], "LISTEN_PID=", strlen ("LISTEN_PID=")) != 0 &&
>> - strncmp (environ[i], "LISTEN_FDS=", strlen ("LISTEN_FDS=")) != 0) {
>> + strncmp (environ[i], "LISTEN_FDS=", strlen ("LISTEN_FDS=")) != 0 &&
>> + strncmp (environ[i], "LISTEN_FDNAMES=",
>> + strlen ("LISTEN_FDNAMES=")) != 0) {
>
> Hmm. Even if we _don't_ expose the ability to set LISTEN_FDNAMES to
> users, we should probably be stripping it from the environment
> (without replacement), rather than just stripping the other two
> LISTEN_* variables.
I'm unsure.
Now, what we strip, mirrors what we provide. That's sustainable because
we revise the set of affected variables *whenever* we need to add a new
variable, in relation to socket activation.
If we start stripping more than what we provide ourselves, it becomes a
game of tag with systemd. I don't know how stable the socket activation
interface is, but I presume further environment variables could be
introduced by systemd. We'd still be catching up the same.
It could work if systemd commandeered an environment variable *prefix*
(aka namespace) for socket activation purposes, and then we could filter
out everything in one go. Unfortunately, LISTEN_ is just too generic for
that. (IOW, I can imagine that systemd will be introducing a few more
LISTEN_ variables in the future, but we still can't filter out
everything LISTEN_*. SYSTEMD_SOCKET_ACTIVATION_ would have been a much
better choice for systemd. Hindsight is 20/20.)
> Which might be worth doing in a separate patch,
> in case it has to be backported across different versions of libnbd.
>
> But overall, I agree with exposing the ability for the client to
> programatically set the name they want, whether by this series or by
> my idea of an alternative API that takes the socket name alongside the
> argv; and whether we keep to our 32-byte [[:alnum:]] limit or instead
> allow for a larger name up to 255 bytes in the regex range notation by
> ASCII byte value [\x20-\x39\x41-\x7e] (aka [ -9;-~], or
> [^\x00-\x1f\x7f-\xff]).
>
Laszlo
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
