Re: [pve-devel] [PATCH common v3 1/2] systemd: add sd_notify() helper

[Fiona Ebner <f.ebner@proxmox.com>]

Am 28.10.25 um 3:58 PM schrieb Wolfgang Bumiller:
> On Tue, Oct 07, 2025 at 02:25:00PM +0200, Fiona Ebner wrote:
>> @@ -282,4 +285,32 @@ sub write_ini {
>>      file_set_contents($filename, $content);
>>  }
>>  
>> +# This is a pure Perl reimplementation of systemd's sd_notify() as defined in systemd/sd-daemon.h
>> +sub sd_notify {
>> +    my ($unset_environment, $state) = @_;
>> +
>> +    my $socket_path = $ENV{NOTIFY_SOCKET};
> 
> Technically this could be an abstract socket. Should be enough to just
> 
>     $socket_path =~ s/^@/\0/;

Will do in v4!

> 
>> +
>> +    my $socket = IO::Socket::UNIX->new(
>> +        Type => SOCK_DGRAM(),
>> +        Peer => $socket_path,
>> +    ) or die "unable to connect to socket $socket_path to notify systemd - $IO::Socket::errstr\n";
>> +
>> +    # we won't be reading from the socket
>> +    $socket->shutdown(SHUT_RD);
>> +
>> +    my $sent = 0;
>> +    my $total = length($state);
>> +    while ($sent < $total) {
>> +        my $res = $socket->send($state);
>> +        die "sending to $socket_path failed - $!" if !$res && $! != EINTR;
>> +        $sent += $res if $res;
> 
> ^ This is a datagram socket. Systemd expects a single datagram.
> The code sort of makes it look like you're trying doing a `write_all()`
> style send (without actually changing what it sent in between calls
> which wouldn't return zero).
> Trying to continue sending in a fragmented way won't work anyway.
> (Otherwise it would be rather cumbersome, since the protocol also allows
> adding things like file descriptors to store in the fd registry; data
> and metadata need to come in one nice bundle)
> 
> The example code in the referenced man page errors out with `-EPROTO` if
> the length does not match, so we could do that as well.
> 
> So basically, only an EINTR loop makes sense here.

Yes, I messed that up. Thank you for the explanation and suggestions!

> 
>> +    }
>> +    $socket->flush();
> 
> This should not be necessary, this is not buffered I/O.

Ack!

>> +
>> +    close($socket);
>> +
>> +    delete($ENV{NOTIFY_SOCKET}) if $unset_environment;
> 
> Why is this part of this function, though?

Because I wanted to match the signature of the original sd_notify(), but
actually it already doesn't match because of the return value. I guess
I'll drop it for now since the examples in 'man 3 sd_notify' also don't
have it. If a future caller needs it, it could also unset the variable
itself.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel