[pve-devel] [PATCH v2 container 1/2] Add module for reading state changes from monitor socket

[pve-devel] [PATCH v2 container 1/2] Add module for reading state changes from monitor socket

[Fabian Ebner]

Will be used to monitor state changes on container startup.

Co-developed-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

New in v2.

I hard-coded the name of the abstract UNIX socket instead of
trying to re-implement lxc/monitor.c's lxc_monitor_sock_name
function. For us, is it true that the lxcpath is always '/varl/lib/lxc'?
Otherwise this won't always work.

 src/PVE/LXC/Makefile   |  1 +
 src/PVE/LXC/Monitor.pm | 92 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)
 create mode 100644 src/PVE/LXC/Monitor.pm

diff --git a/src/PVE/LXC/Makefile b/src/PVE/LXC/Makefile
index f4f4dc1..a190260 100644
--- a/src/PVE/LXC/Makefile
+++ b/src/PVE/LXC/Makefile
@@ -4,6 +4,7 @@ SOURCES= \
 	Config.pm \
 	Create.pm \
 	Migrate.pm \
+	Monitor.pm \
 	Setup.pm \
 	Tools.pm
 
diff --git a/src/PVE/LXC/Monitor.pm b/src/PVE/LXC/Monitor.pm
new file mode 100644
index 0000000..3a592b5
--- /dev/null
+++ b/src/PVE/LXC/Monitor.pm
@@ -0,0 +1,92 @@
+# LXC monitor socket
+
+package PVE::LXC::Monitor;
+
+use strict;
+use warnings;
+
+use IO::Socket::UNIX;
+use Socket qw(SOCK_STREAM);
+use POSIX qw(NAME_MAX);
+
+use constant {
+    STATE_STOPPED => 0,
+    STATE_STARTING => 1,
+    STATE_RUNNING => 2,
+    STATE_STOPPING => 3,
+    STATE_ABORTING => 4,
+    STATE_FREEZING => 5,
+    STATE_FROZEN => 6,
+    STATE_THAWED => 7,
+    MAX_STATE => 8,
+};
+
+my $LXC_MSG_SIZE = length(pack('I! Z'.(NAME_MAX+1).' x![I] I', 0, "", 0));
+# Unpack an lxc_msg struct.
+my sub _unpack_lxc_msg($) {
+    my ($packet) = @_;
+
+    # struct lxc_msg {
+    #     lxc_msg_type_t type;
+    #     char name[NAME_MAX+1];
+    #     int value;
+    # };
+
+    my ($type, $name, $value) = unpack('I!Z'.(NAME_MAX+1).'I!', $packet);
+
+    if ($type == 0) {
+	$type = 'STATE';
+    } elsif ($type == 1) {
+	$type = 'PRIORITY';
+    } elsif ($type == 2) {
+	$type = 'EXITCODE';
+    } else {
+	warn "unsupported lxc message type $type received\n";
+	$type = undef;
+    }
+
+    return ($type, $name, $value);
+}
+
+# Opens the monitor socket
+#
+# Dies on errors
+sub get_monitor_socket {
+    my $socket = IO::Socket::UNIX->new(
+	Type => SOCK_STREAM(),
+	# assumes that lxcpath is '/var/lib/lxc', the hex part is a hash of the lxcpath
+	Peer => "\0lxc/ad055575fe28ddd5//var/lib/lxc",
+    );
+    if (!defined($socket)) {
+	die "failed to connect to monitor socket: $!\n";
+    }
+
+    return $socket;
+}
+
+# Read an lxc message from a socket.
+#
+# Returns undef on EOF
+# Otherwise returns a (type, vmid, value) tuple.
+#
+# The returned 'type' currently can be 'STATE', 'PRIORITY' or 'EXITSTATUS'.
+sub read_lxc_message($) {
+    my ($socket) = @_;
+
+    my $msg;
+    my $got = recv($socket, $msg, $LXC_MSG_SIZE, 0)
+	// die "failed to read from state socket: $!\n";
+
+    if (length($msg) == 0) {
+	return undef;
+    }
+
+    die "short read on state socket ($LXC_MSG_SIZE != ".length($msg).")\n"
+	if length($msg) != $LXC_MSG_SIZE;
+
+    my ($type, $name, $value) = _unpack_lxc_msg($msg);
+
+    return ($type, $name, $value);
+}
+
+1;
-- 
2.20.1

[pve-devel] [PATCH v2 container 2/2] Improve feedback for startup

[Fabian Ebner]

Since it was necessary to switch to 'Type=Simple' in the systemd
service, see 545d6f0a13ac2bf3a8d3f224c19c0e0def12116d,
'systemctl start' would not wait for the 'lxc-start' command anymore.
Thus every container start was reported as a success and the 'post-start'
hook would trigger immediately after the 'systemctl start' command.

Use the monitor socket to get the necessary information and detect
startup failure, and only run the 'post-start' hookscript after
the container is effectively running. If something goes wrong
with the monitor socket, for example if lxc-monitord is not running,
fall back to the old behavior.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v1:
    * use monitor socket directly instead of forking off an lxc-monitor process
    * use run_with_timeout helper
    * warn instead of die on unexpected message

 src/PVE/LXC.pm | 40 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index db5b8ca..370adda 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -32,6 +32,7 @@ use PVE::LXC::Config;
 use PVE::GuestHelpers qw(safe_string_ne safe_num_ne safe_boolean_ne);
 use PVE::LXC::Tools;
 use PVE::LXC::CGroup;
+use PVE::LXC::Monitor;
 
 use Time::HiRes qw (gettimeofday);
 my $have_sdn;
@@ -2191,10 +2192,47 @@ sub vm_start {
 
     PVE::Storage::activate_volumes($storage_cfg, $vollist);
 
+    my $monitor_socket = eval { PVE::LXC::Monitor::get_monitor_socket(); };
+    warn $@ if $@;
+
+    my $monitor_state_change = sub {
+	die "no monitor socket" if !defined($monitor_socket);
+
+	while (1) {
+	    my ($type, $name, $value) = PVE::LXC::Monitor::read_lxc_message($monitor_socket);
+
+	    die "monitor socket EOF" if !defined($type);
+
+	    next if $name ne "$vmid" || $type ne 'STATE';
+
+	    if ($value eq PVE::LXC::Monitor::STATE_STARTING) {
+		alarm(0); # don't timeout after seeing the starting state
+	    } elsif ($value eq PVE::LXC::Monitor::STATE_ABORTING ||
+		     $value eq PVE::LXC::Monitor::STATE_STOPPING ||
+		     $value eq PVE::LXC::Monitor::STATE_STOPPED) {
+		return 0;
+	    } elsif ($value eq PVE::LXC::Monitor::STATE_RUNNING) {
+		return 1;
+	    } else {
+		warn "unexpected message from monitor socket - " .
+		     "type: '$type' - value: '$value'\n";
+	    }
+	}
+    };
+
     my $cmd = ['systemctl', 'start', "pve-container\@$vmid"];
 
     PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'pre-start', 1);
-    eval { PVE::Tools::run_command($cmd); };
+    eval {
+	PVE::Tools::run_command($cmd);
+
+	my $success = eval { PVE::Tools::run_with_timeout(10, $monitor_state_change); };
+	if (my $err = $@) {
+	    warn "problem with monitor socket: $err - continuing anyway\n";
+	} elsif (!$success) {
+	    die "startup for container '$vmid' failed\n";
+	}
+    };
     if (my $err = $@) {
 	unlink $skiplock_flag_fn;
 	die $err;
-- 
2.20.1

[pve-devel] applied-series: [PATCH v2 container 1/2] Add module for reading state changes from monitor socket

[Thomas Lamprecht]

On 08.09.20 13:58, Fabian Ebner wrote:
> Will be used to monitor state changes on container startup.
> 
> Co-developed-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> New in v2.
> 
> I hard-coded the name of the abstract UNIX socket instead of
> trying to re-implement lxc/monitor.c's lxc_monitor_sock_name
> function. For us, is it true that the lxcpath is always '/varl/lib/lxc'?
> Otherwise this won't always work.
> 
>  src/PVE/LXC/Makefile   |  1 +
>  src/PVE/LXC/Monitor.pm | 92 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 93 insertions(+)
>  create mode 100644 src/PVE/LXC/Monitor.pm
> 
>

applied series, thanks! Wolfgang may improve his variant, e.g., by patching lxc, to avoid
a runtime dependency on the lxc monitor, but until that works out this is 10000x times
better that the current status quo.