all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Hannes Laimer <h.laimer@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH pve-cluster 10/12] cfs: add perl client for the change notification socket
Date: Thu, 22 Oct 2026 16:22:03 +0200	[thread overview]
Message-ID: <20261022142205.930536-11-h.laimer@proxmox.com> (raw)
In-Reply-To: <20261022142205.930536-1-h.laimer@proxmox.com>

Daemons that want to react to cluster-wide config changes all need to
connect to the pmxcfs notification socket, subscribe with path
templates, wait for events, and recover when pmxcfs restarts. A shared
client module handles that, so every consumer gets the reconnect
handling right instead of reimplementing it.

The client retries a lost connection and a failed connect on its own,
with a growing backoff between failed attempts. An error reply from
the daemon or an unsupported protocol version makes the call die
instead, since a retry would get the same answer. Two processes
reading one stream would split its lines between them, so a forked
child opens a connection of its own.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 debian/pve-cluster.install    |   1 +
 src/PVE/Cluster/Makefile      |   2 +-
 src/PVE/Cluster/Watch.pm      | 325 ++++++++++++++++++++++++++++++++++
 src/test/Makefile             |   6 +-
 src/test/watch_client_test.pl | 229 ++++++++++++++++++++++++
 5 files changed, 561 insertions(+), 2 deletions(-)
 create mode 100644 src/PVE/Cluster/Watch.pm
 create mode 100644 src/test/watch_client_test.pl

diff --git a/debian/pve-cluster.install b/debian/pve-cluster.install
index f66cd06..77e3244 100644
--- a/debian/pve-cluster.install
+++ b/debian/pve-cluster.install
@@ -5,4 +5,5 @@ usr/lib/
 usr/share/man/man8/pmxcfs.8
 usr/share/perl5/PVE/Cluster.pm
 usr/share/perl5/PVE/Cluster/IPCConst.pm
+usr/share/perl5/PVE/Cluster/Watch.pm
 usr/share/perl5/PVE/IPCC.pm
diff --git a/src/PVE/Cluster/Makefile b/src/PVE/Cluster/Makefile
index 3f920cb..7beb976 100644
--- a/src/PVE/Cluster/Makefile
+++ b/src/PVE/Cluster/Makefile
@@ -1,6 +1,6 @@
 PVEDIR=$(DESTDIR)/usr/share/perl5/PVE
 
-SOURCES=IPCConst.pm Setup.pm
+SOURCES=IPCConst.pm Setup.pm Watch.pm
 
 .PHONY: install
 install: $(SOURCES)
diff --git a/src/PVE/Cluster/Watch.pm b/src/PVE/Cluster/Watch.pm
new file mode 100644
index 0000000..dde8711
--- /dev/null
+++ b/src/PVE/Cluster/Watch.pm
@@ -0,0 +1,325 @@
+package PVE::Cluster::Watch;
+
+use strict;
+use warnings;
+
+use IO::Select;
+use IO::Socket::UNIX;
+use JSON;
+use Socket qw(SOCK_STREAM MSG_NOSIGNAL);
+use Time::HiRes qw(time sleep);
+
+my $default_socket = '/run/pve-cluster/pmxcfs.sock';
+my $protocol_version = 1;
+my $io_timeout = 10;
+my $min_backoff = 1;
+my $max_backoff = 30;
+
+sub new {
+    my ($class, %param) = @_;
+
+    my $self = bless {
+        socket_path => $param{socket} // $default_socket,
+        patterns => {},
+        pid => $$,
+    }, $class;
+    $self->reset();
+
+    return $self;
+}
+
+# Disconnected, with a connect due right away.
+sub reset {
+    my ($self) = @_;
+
+    $self->{sock} = undef;
+    $self->{buf} = '';
+    $self->{queue} = [];
+    $self->{established} = 0;
+    $self->{next_connect} = 0;
+    $self->{backoff} = $min_backoff;
+    $self->{warned} = 0;
+
+    return;
+}
+
+# Two processes reading one connection would split its lines between them,
+# so a forked child connects on its own, as IPCC does. It closes the
+# inherited copy without a shutdown, which would cut off the parent as well.
+# A child that keeps running should close the watch, otherwise that copy
+# stays open until the child next calls a method of the watch.
+sub check_fork {
+    my ($self) = @_;
+
+    return if $self->{pid} == $$;
+
+    $self->{sock}->close() if $self->{sock};
+    $self->{pid} = $$;
+    $self->reset();
+
+    return;
+}
+
+# Takes effect when connected and is replayed on every reconnect. Dies if
+# the daemon refuses it. A connection lost while sending it is dropped, and
+# the next wait reconnects.
+sub subscribe {
+    my ($self, %param) = @_;
+
+    $self->check_fork();
+
+    my $previous = $self->{patterns};
+    $self->{patterns} = { ($param{patterns} // {})->%* };
+
+    return if !$self->{sock};
+
+    my $error = eval { $self->send_subscription() };
+    if ($@) {
+        chomp(my $reason = $@);
+        $self->disconnect($reason) if $self->{sock};
+        return;
+    }
+    # the daemon keeps serving the previous subscription, so keep its
+    # patterns for the next reconnect
+    if (defined($error)) {
+        $self->{patterns} = $previous;
+        die "subscription refused: $error\n";
+    }
+
+    return;
+}
+
+sub connected {
+    my ($self) = @_;
+
+    $self->check_fork();
+
+    return defined($self->{sock});
+}
+
+# A reconnect replaces the socket, possibly under the same number, so a
+# caller watching it has to look it up again after every wait. It is undef
+# while disconnected, and only wait reconnects.
+sub fd {
+    my ($self) = @_;
+
+    $self->check_fork();
+
+    return $self->{sock} ? fileno($self->{sock}) : undef;
+}
+
+sub close {
+    my ($self) = @_;
+
+    $self->check_fork();
+
+    $self->{sock}->close() if $self->{sock};
+    $self->{sock} = undef;
+    $self->{established} = 0;
+    $self->{buf} = '';
+
+    return;
+}
+
+# Returns the queued events as soon as there are any, or none once $timeout
+# seconds have passed or a signal has arrived. A due reconnect also happens
+# here and is the one way a zero timeout can block. Dies if the daemon
+# refuses the handshake, since a retry would not change that.
+sub wait {
+    my ($self, $timeout) = @_;
+
+    $self->check_fork();
+
+    my $deadline = time() + ($timeout // 0);
+
+    while (1) {
+        if (scalar($self->{queue}->@*)) {
+            my @events = $self->{queue}->@*;
+            $self->{queue} = [];
+            return @events;
+        }
+
+        my $now = time();
+        if (!$self->{sock}) {
+            if ($now >= $self->{next_connect}) {
+                $self->connect();
+                next;
+            }
+            my $until = $self->{next_connect} < $deadline ? $self->{next_connect} : $deadline;
+            return if $until <= $now;
+            sleep($until - $now);
+            # a signal cut the sleep short, and a caller whose handler only
+            # sets a flag needs control back to act on it
+            return if time() < $until;
+            next;
+        }
+
+        my $remaining = $deadline - $now;
+        $remaining = 0 if $remaining < 0;
+
+        return if !IO::Select->new($self->{sock})->can_read($remaining);
+        $self->read_messages();
+    }
+}
+
+sub connect {
+    my ($self) = @_;
+
+    # connect blocks while the daemon's backlog is full, as when it cannot
+    # accept for a while. The timeout bounds that, and a failed attempt is
+    # retried later
+    my $sock = IO::Socket::UNIX->new(
+        Type => SOCK_STREAM,
+        Peer => $self->{socket_path},
+        Timeout => $io_timeout,
+    );
+    if (!$sock) {
+        $self->connect_failed("connect to $self->{socket_path} failed: $!");
+        return;
+    }
+
+    $self->{sock} = $sock;
+
+    my $error = eval { $self->handshake() };
+    if (my $reason = $@) {
+        $self->close();
+        $self->connect_failed("handshake with $self->{socket_path} failed: $reason");
+        return;
+    }
+    if (defined($error)) {
+        $self->close();
+        $self->delay_retry();
+        die "handshake with $self->{socket_path} refused: $error\n";
+    }
+
+    $self->{established} = 1;
+    $self->{backoff} = $min_backoff;
+    $self->{warned} = 0;
+
+    return;
+}
+
+sub handshake {
+    my ($self) = @_;
+
+    my ($hello, $error) = $self->request('hello');
+    return $error if defined($error);
+
+    my $version = ref($hello) eq 'HASH' ? $hello->{protocol} : undef;
+    return "unsupported protocol version " . ($version // 'unknown')
+        if !defined($version) || $version != $protocol_version;
+
+    return $self->send_subscription();
+}
+
+sub connect_failed {
+    my ($self, $msg) = @_;
+
+    if (!$self->{warned}) {
+        chomp $msg;
+        warn "$msg, retrying\n";
+        $self->{warned} = 1;
+    }
+
+    $self->delay_retry();
+
+    return;
+}
+
+sub delay_retry {
+    my ($self) = @_;
+
+    $self->{next_connect} = time() + $self->{backoff};
+    $self->{backoff} *= 2;
+    $self->{backoff} = $max_backoff if $self->{backoff} > $max_backoff;
+
+    return;
+}
+
+# A connection lost before its handshake completed is reported by the
+# connect attempt, so a daemon that accepts and then closes causes one
+# warning, not two.
+sub disconnect {
+    my ($self, $reason) = @_;
+
+    my $established = $self->{established};
+    $self->close();
+    return if !$established;
+
+    warn "notification socket $reason, reconnecting\n";
+    $self->{next_connect} = time();
+    $self->{backoff} = $min_backoff;
+
+    return;
+}
+
+sub send_subscription {
+    my ($self) = @_;
+
+    my (undef, $error) = $self->request('subscribe', { patterns => $self->{patterns} });
+
+    return $error;
+}
+
+# Sends a request and returns its reply as (data, error), queueing any events
+# that arrive meanwhile. Dies when no reply comes in time or the connection
+# is lost.
+sub request {
+    my ($self, $command, $args) = @_;
+
+    my $line = encode_json({ command => $command, ($args ? (args => $args) : ()) }) . "\n";
+    my $written = 0;
+    while ($written < length($line)) {
+        my $len = send($self->{sock}, substr($line, $written), MSG_NOSIGNAL);
+        next if !defined($len) && $!{EINTR};
+        die "write failed: $!\n" if !defined($len);
+        $written += $len;
+    }
+
+    my $deadline = time() + $io_timeout;
+    while (1) {
+        my $remaining = $deadline - time();
+        die "timeout waiting for reply to '$command'\n" if $remaining <= 0;
+        # bounds the wait, since the read itself would block for as long as
+        # the daemon stays silent
+        next if !IO::Select->new($self->{sock})->can_read($remaining);
+
+        for my $msg ($self->read_messages()) {
+            return ($msg->{ok}, undef) if exists $msg->{ok};
+            return (undef, $msg->{error} // 'unknown error') if exists $msg->{error};
+        }
+        die "connection lost while waiting for reply to '$command'\n" if !$self->{sock};
+    }
+}
+
+sub read_messages {
+    my ($self) = @_;
+
+    my $len;
+    do {
+        # appended to what the last read left unfinished
+        $len = sysread($self->{sock}, $self->{buf}, 65536, length($self->{buf}));
+    } while (!defined($len) && $!{EINTR});
+    if (!$len) {
+        $self->disconnect(defined($len) ? 'closed' : "read failed: $!");
+        return;
+    }
+
+    my @replies;
+    while ($self->{buf} =~ s/^([^\n]*)\n//) {
+        my $msg = eval { decode_json($1) };
+        if (ref($msg) ne 'HASH') {
+            $self->disconnect('sent a malformed line');
+            return;
+        }
+        if (my $event = $msg->{event}) {
+            push $self->{queue}->@*, $event;
+        } else {
+            push @replies, $msg;
+        }
+    }
+
+    return @replies;
+}
+
+1;
diff --git a/src/test/Makefile b/src/test/Makefile
index cdd37d0..7b36fca 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -4,7 +4,7 @@ cpgtest: cpgtest.c
 	gcc -Wall cpgtest.c $(shell pkg-config --cflags --libs libcpg libqb) -o cpgtest
 
 .PHONY: check install clean distclean
-check: corosync-parser-test test-mac-prefix
+check: corosync-parser-test test-mac-prefix watch-client-test
 
 .PHONY: corosync-parser-test
 corosync-parser-test:
@@ -14,5 +14,9 @@ corosync-parser-test:
 test-mac-prefix:
 	perl test_mac_prefix.pl
 
+.PHONY: watch-client-test
+watch-client-test:
+	perl watch_client_test.pl
+
 distclean: clean
 clean:
diff --git a/src/test/watch_client_test.pl b/src/test/watch_client_test.pl
new file mode 100644
index 0000000..ef8159e
--- /dev/null
+++ b/src/test/watch_client_test.pl
@@ -0,0 +1,229 @@
+#!/usr/bin/perl
+
+use lib '..';
+
+use strict;
+use warnings;
+
+use File::Temp qw(tempdir);
+use IO::Socket::UNIX;
+use JSON;
+use Socket qw(SOCK_STREAM);
+use Test::More;
+use Time::HiRes qw(sleep time);
+
+use PVE::Cluster::Watch;
+
+my $dir = tempdir(CLEANUP => 1);
+my $path = "$dir/sock";
+
+my $listener = IO::Socket::UNIX->new(Type => SOCK_STREAM, Local => $path, Listen => 1)
+    or die "listen failed: $!\n";
+
+# A stand-in for pmxcfs that serves ten connections in a row. The fourth
+# refuses the subscription, the fifth hangs up after answering the hello,
+# the seventh refuses a second subscription on a live connection and the
+# last three hang up before answering anything. The others answer the
+# handshake, send the resync, mirror the subscription back inside two
+# events and hang up, which forces the client through its reconnect path.
+my $server = fork() // die "fork failed: $!\n";
+if (!$server) {
+    for my $round (1 .. 10) {
+        my $conn = $listener->accept() or die "accept failed: $!\n";
+        if ($round >= 8) {
+            close($conn);
+            next;
+        }
+        my $subscribed = 0;
+        while (my $line = <$conn>) {
+            my $req = decode_json($line);
+            if ($req->{command} eq 'hello') {
+                print $conn encode_json({ ok => { protocol => 1 } }), "\n";
+            } elsif ($round == 5) {
+                last;
+            } elsif ($req->{command} eq 'subscribe') {
+                if ($round == 4) {
+                    print $conn encode_json({ error => "pattern 'guest': no good" }), "\n";
+                    last;
+                }
+                if ($round == 7 && $subscribed) {
+                    print $conn encode_json({ error => "pattern 'bad': no good" }), "\n";
+                    next;
+                }
+                $subscribed = 1;
+                my $patterns = $req->{args}->{patterns};
+                print $conn encode_json({ ok => undef }), "\n";
+                print $conn encode_json({ event => { type => 'resync' } }), "\n";
+                next if $round == 7;
+                print $conn encode_json({
+                    event => {
+                        type => 'write',
+                        path => 'x',
+                        params =>
+                            { map { $_ => [{ template => $patterns->{$_} }] } keys %$patterns },
+                    },
+                    }),
+                    "\n";
+                print $conn encode_json({
+                    event => {
+                        type => 'rename',
+                        path => 'old',
+                        to => join(',', sort keys %$patterns),
+                    },
+                    }),
+                    "\n";
+                last;
+            } else {
+                print $conn encode_json({ error => "unknown command '$req->{command}'" }), "\n";
+            }
+        }
+        close($conn);
+    }
+    exit(0);
+}
+close($listener);
+
+my $collect = sub {
+    my ($watch, $count, $timeout) = @_;
+    my @events;
+    my $deadline = time() + 10;
+    while (scalar(@events) < $count && time() < $deadline) {
+        push @events, $watch->wait($timeout);
+        sleep(0.05) if !$timeout;
+    }
+    return \@events;
+};
+
+my $watch = PVE::Cluster::Watch->new(socket => $path);
+ok(!$watch->connected(), 'not connected before the first wait');
+
+my $template = 'nodes/{node}/qemu-server/{vmid}.conf';
+$watch->subscribe(patterns => { guest => $template });
+
+my $write = {
+    type => 'write',
+    path => 'x',
+    params => { guest => [{ template => $template }] },
+};
+my $rename = { type => 'rename', path => 'old', to => 'guest' };
+my $expected = [{ type => 'resync' }, $write, $rename];
+
+is_deeply($collect->($watch, 3, 1), $expected, 'connect gives a resync followed by the events');
+ok(defined($watch->fd()), 'a descriptor is available while connected');
+
+$watch->{pid} = -1; # pretend the object was inherited across a fork
+is_deeply(
+    $collect->($watch, 3, 0),
+    $expected,
+    'a forked child gets a connection of its own, a zero timeout drains it',
+);
+
+is_deeply(
+    $collect->($watch, 3, 1),
+    $expected,
+    'reconnect after hangup replays the subscription and gives a resync again',
+);
+
+# the server has hung up on that connection by now. Writing to it must
+# neither raise SIGPIPE nor die, and the next wait reconnects
+sleep(0.2);
+$watch->subscribe(patterns => { guest => $template });
+ok(!$watch->connected(), 'a subscription on a lost connection drops it');
+
+eval { $watch->wait(1) };
+like($@, qr/refused: pattern 'guest': no good/, 'a refused subscription makes the wait die');
+ok(!$watch->connected(), 'and its connection is closed');
+
+# a handshake cut after the hello, rounds five and six
+{
+    my @warnings;
+    local $SIG{__WARN__} = sub { push @warnings, $_[0] };
+    my $half = PVE::Cluster::Watch->new(socket => $path);
+    $half->subscribe(patterns => { guest => $template });
+    is_deeply(
+        $collect->($half, 3, 1),
+        $expected,
+        'a connect that is cut after the hello is retried and gives a resync',
+    );
+    is(scalar(@warnings), 1, 'and the cut handshake is reported');
+}
+
+# a refused subscription on a live connection, round seven. The connection
+# stays up after the refusal, as it does with pmxcfs
+{
+    my $live = PVE::Cluster::Watch->new(socket => $path);
+    $live->subscribe(patterns => { guest => $template });
+    is_deeply(
+        $collect->($live, 1, 1),
+        [{ type => 'resync' }],
+        'connected with the first subscription',
+    );
+    is_deeply(
+        [$live->request('bogus')],
+        [undef, "unknown command 'bogus'"],
+        'an error reply outside the handshake comes back as the error',
+    );
+    eval { $live->subscribe(patterns => { bad => 'x/{y}' }) };
+    like($@, qr/subscription refused: pattern 'bad': no good/, 'a refusal makes subscribe die');
+    is_deeply($live->{patterns}, { guest => $template }, 'and the previous patterns stay');
+    ok($live->connected(), 'on a connection that stays up');
+}
+
+# a daemon that accepts and hangs up immediately, rounds eight to ten
+{
+    my @warnings;
+    local $SIG{__WARN__} = sub { push @warnings, $_[0] };
+    my $cut = PVE::Cluster::Watch->new(socket => $path);
+    $cut->subscribe(patterns => { guest => $template });
+    my @none = $cut->wait(5);
+    is(scalar(@none), 0, 'nothing arrives from a daemon that hangs up immediately');
+    is(scalar(@warnings), 1, 'which is reported once and retried with backoff');
+    ok(!$cut->connected(), 'and leaves the client disconnected');
+}
+
+# a signal that cuts a wait short while the client waits for its next
+# connect attempt
+{
+    my @warnings;
+    local $SIG{__WARN__} = sub { push @warnings, $_[0] };
+    my $flagged = 0;
+    local $SIG{USR1} = sub { $flagged = 1 };
+
+    my $parent = $$;
+    my $signaller = fork() // die "fork failed: $!\n";
+    if (!$signaller) {
+        sleep(0.3);
+        kill 'USR1', $parent;
+        exit(0);
+    }
+
+    my $gone = PVE::Cluster::Watch->new(socket => "$dir/gone");
+    my $started = time();
+    my @none = $gone->wait(3);
+    my $elapsed = time() - $started;
+    waitpid($signaller, 0);
+
+    is(scalar(@none), 0, 'a wait cut short by a signal returns no events');
+    ok($flagged, 'the handler of the caller has run');
+    # a signal that lands after the first backoff sleep ended only cuts the
+    # second one short, so the bound has to sit above one backoff and still
+    # below the deadline
+    ok(
+        $elapsed < 2.5,
+        sprintf('and the wait gave up after %.2fs, short of its deadline', $elapsed),
+    );
+    is(scalar(@warnings), 1, 'the socket that is not there is reported once');
+}
+
+# a shortfall in connections must fail the test, not hang it
+local $SIG{ALRM} = sub { kill 'KILL', $server; die "fake server did not finish\n" };
+alarm(30);
+waitpid($server, 0);
+alarm(0);
+is($?, 0, 'fake server ran all rounds cleanly');
+
+my @none = $watch->wait(1);
+is(scalar(@none), 0, 'no events once the server is gone');
+ok(!$watch->connected(), 'disconnected after the server went away');
+
+done_testing();
-- 
2.47.3





  parent reply	other threads:[~2026-10-22 14:24 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-22 14:21 [PATCH cluster 00/12] pmxcfs: add a change notification socket Hannes Laimer
2026-10-22 14:21 ` [PATCH pve-cluster 01/12] buildsys: add rust workspace under src/rust Hannes Laimer
2026-10-22 14:21 ` [PATCH pve-cluster 02/12] rust: notify: add path templates with placeholder capture Hannes Laimer
2026-10-22 14:21 ` [PATCH pve-cluster 03/12] rust: notify: add the wire protocol Hannes Laimer
2026-10-22 14:21 ` [PATCH pve-cluster 04/12] rust: notify: add subscription matching Hannes Laimer
2026-10-22 14:21 ` [PATCH pve-cluster 05/12] rust: notify: add the socket server Hannes Laimer
2026-10-22 14:21 ` [PATCH pve-cluster 06/12] rust: ffi: add C ABI staticlib for pmxcfs Hannes Laimer
2026-10-22 14:22 ` [PATCH pve-cluster 07/12] pmxcfs: memdb: add change notification hook Hannes Laimer
2026-10-22 14:22 ` [PATCH pve-cluster 08/12] buildsys: link pmxcfs against the rust notify staticlib Hannes Laimer
2026-10-22 14:22 ` [PATCH pve-cluster 09/12] pmxcfs: notify: emit change events over the notification socket Hannes Laimer
2026-10-22 14:22 ` Hannes Laimer [this message]
2026-10-22 14:22 ` [PATCH pve-cluster 11/12] cfs: add hook registry for change notification consumers Hannes Laimer
2026-10-22 14:22 ` [PATCH pve-cluster 12/12] cfs: add runner executing cluster change hooks in children Hannes Laimer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261022142205.930536-11-h.laimer@proxmox.com \
    --to=h.laimer@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal