public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Wolfgang Bumiller <w.bumiller@proxmox.com>
Subject: Re: [pve-devel] [RFC container] Improve feedback for startup
Date: Mon, 7 Sep 2020 18:32:32 +0200	[thread overview]
Message-ID: <e4f128e8-ac13-4553-4086-fa2a8028f935@proxmox.com> (raw)
In-Reply-To: <20200827084404.o2aizoasd6r7xl4d@olga.proxmox.com>

On 27.08.20 10:44, Wolfgang Bumiller wrote:
> On Thu, Aug 20, 2020 at 11:36:39AM +0200, Thomas Lamprecht wrote:
>> On 19.08.20 12:30, Fabian Ebner wrote:
>>> Since it was necessary to switch to 'Type=Simple' in the systemd
>>> service (see 545d6f0a13ac2bf3a8d3f224c19c0e0def12116d ),
>>> 'systemctl start pve-container@ID' 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 'lxc-monitor' 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, fall back to the old behavior.
>>>
>>> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
>>> ---
>>>  src/PVE/LXC.pm | 36 +++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 35 insertions(+), 1 deletion(-)
>>>
>>
>> appreciate the effort!
>> We could also directly connect to /run/lxc/var/lib/lxc/monitor-fifo (or the abstract
>> unix socket, but not much gained/difference here) of the lxc-monitord which publishes
>> all state changes and unpack the new state [0] directly.
>>
>> [0] https://github.com/lxc/lxc/blob/8bdacc22a48f9c09902a1d2febd71439cb38c082/src/lxc/state.h#L10
>>
>> @Wolfgang, what do you think?
> 
> Just tested adding a state client to our Command.pm directly, seems to
> work, so we would depend neither on lxc-monitor nor lxc-monitord.
> 
> Example & code follow below. The only issue with it is that we'd need to
> retry connecting to the command socket a few times since we don't know
> when it becomes available, but that shouldn't be too bad IMO.
> 
> [..snip..]

With below I never get the initial stopped -> running edge, though.
I can monitor the CT getting stopped, but not the other way around.
Adding extra code to check if the CTs running to abort the recv would
feel like this missing the point a bit...

> 
> Usage example:
> 
>     use PVE::LXC::Command;
> 
>     my $sock = PVE::LXC::Command::get_state_client(404);
>     die "not running\n" if !defined($sock);
> 
>     while (1) {
>         my ($type, $name, $value) = PVE::LXC::Command::read_lxc_message($sock);
>         last if !defined($type);
>         print("$name: $type => $value\n");
>     }
> 
> Patch for Command.pm:
> 
> ---8<---
> From 6ac578ef889a3a9c8aefc4f05215b4ec66049546 Mon Sep 17 00:00:00 2001
> From: Wolfgang Bumiller <w.bumiller@proxmox.com>
> Date: Thu, 27 Aug 2020 10:31:06 +0200
> Subject: [PATCH container] command: add state client functions
> 
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>  src/PVE/LXC/Command.pm | 91 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 91 insertions(+)
> 
> diff --git a/src/PVE/LXC/Command.pm b/src/PVE/LXC/Command.pm
> index beed890..6df767d 100644
> --- a/src/PVE/LXC/Command.pm
> +++ b/src/PVE/LXC/Command.pm
> @@ -11,20 +11,36 @@ use warnings;
>  
>  use IO::Socket::UNIX;
>  use Socket qw(SOCK_STREAM SOL_SOCKET SO_PASSCRED);
> +use POSIX qw(NAME_MAX);
>  
>  use base 'Exporter';
>  
>  use constant {
> +    LXC_CMD_GET_STATE => 3,
>      LXC_CMD_GET_CGROUP => 6,
> +    LXC_CMD_ADD_STATE_CLIENT => 10,
>      LXC_CMD_FREEZE => 15,
>      LXC_CMD_UNFREEZE => 16,
>      LXC_CMD_GET_LIMITING_CGROUP => 19,
>  };
>  
> +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,
> +};
> +
>  our @EXPORT_OK = qw(
>      raw_command_transaction
>      simple_command
>      get_cgroup_path
> +    get_state_client
>  );
>  
>  # Get the command socket for a container.
> @@ -81,6 +97,33 @@ my sub _unpack_lxc_cmd_rsp($) {
>      return ($ret, $len);
>  }
>  
> +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);
> +}
> +
>  # Send a complete packet:
>  my sub _do_send($$) {
>      my ($sock, $data) = @_;
> @@ -206,4 +249,52 @@ sub unfreeze($$) {
>      return $res;
>  }
>  
> +# Add this command socket as a state client.
> +#
> +# Currently all states are observed.
> +#
> +# Returns undef if the container is not running, dies on errors.
> +sub get_state_client($) {
> +    my ($vmid) = @_;
> +
> +    my $socket = _get_command_socket($vmid)
> +	or return undef;
> +
> +    # For now we want all states (except 'reboots', since we would never see those, reboots would
> +    # use a value of '2' for STATE_RUNNING)
> +    my $states = pack('I!', 2) x MAX_STATE;
> +
> +    my ($res, undef) = raw_command_transaction($socket, LXC_CMD_ADD_STATE_CLIENT, $states);
> +    if ($res != MAX_STATE) {
> +	die "container is currently in unexpected state $res\n";
> +    }
> +
> +    return $socket;
> +}
> +
> +# Read an lxc message from a socket.
> +#
> +# Returns undef on EOF (if lxc exits).
> +# 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;
> 






      reply	other threads:[~2020-09-07 16:32 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-19 10:30 Fabian Ebner
2020-08-20  9:36 ` Thomas Lamprecht
2020-08-27  8:44   ` Wolfgang Bumiller
2020-09-07 16:32     ` Thomas Lamprecht [this message]

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=e4f128e8-ac13-4553-4086-fa2a8028f935@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    --cc=w.bumiller@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal