all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: [pve-devel] applied: [PATCH container v2] Move volume activation to vm_start
Date: Tue, 04 Aug 2020 14:09:01 +0200	[thread overview]
Message-ID: <1596542933.0g19smq090.astroid@nora.none> (raw)
In-Reply-To: <20200714175018.9592-1-s.ivanov@proxmox.com>

On July 14, 2020 7:50 pm, Stoiko Ivanov wrote:
> currently all volumes for a container are activated in the pre-start hook,
> which runs in a separate mount namespace (lxc.monitor.unshare is set to 1
> in our container config). This leads to problems with ZFS:
> * if a pool is imported by this call the filesystems are mounted only inside
>   the containers mount namespace
> 
> by running the volume activation inside vm_start, right before starting the
> container via systemctl the volume activation happens before the unshare.
> 
> The other site where a container is started via systemctl is in
> 'pve-container-stop-wrapper' when a container is rebooted from the inside:
> By activating the volumes in 'lxc-pve-poststop-hook' we avoid to try starting
> a container with an inactive volume (LVM, kRBD), occuring when having a
> mp-addtion pending during such a reboot
> 
> Starting a container manually using lxc-start is usually done for obtaining
> debug-logs (after starting failed with our tooling) - so the potential for
> regression in that case should also be small.
> 
> The $loopdevlist variable is not used anywhere in our codebase since 2015
> (da6298481ea4dfe7d894f42fa105cda015ebe5ce).
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
> v1 -> v2:
> * added the volume activation to lxc-pve-poststop-hook, after the great review
>   from Fabian caught the case of container reboots (huge thanks!)
> * decided to put it in lxc-pve-poststop-hook instead of
>   pve-container-stop-wrapper, since this avoids pulling in quite a few parts
>   of our code-base into the latter, which currently has no imports.
>   AFAICU the only case where this could lead to a problem is if someone uses
>   `touch /var/lib/lxc/$vmid/reboot && systemctl restart pve-container@$vmid`
>   for rebooting containers
> 
> 
>  src/PVE/LXC.pm            | 5 +++++
>  src/lxc-pve-poststop-hook | 6 ++++++
>  src/lxc-pve-prestart-hook | 5 -----
>  3 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
> index f3aca7a..db5b8ca 100644
> --- a/src/PVE/LXC.pm
> +++ b/src/PVE/LXC.pm
> @@ -2186,6 +2186,11 @@ sub vm_start {
>  	close($fh);
>      }
>  
> +    my $storage_cfg = PVE::Storage::config();
> +    my $vollist = PVE::LXC::Config->get_vm_volumes($conf);
> +
> +    PVE::Storage::activate_volumes($storage_cfg, $vollist);
> +
>      my $cmd = ['systemctl', 'start', "pve-container\@$vmid"];
>  
>      PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'pre-start', 1);
> diff --git a/src/lxc-pve-poststop-hook b/src/lxc-pve-poststop-hook
> index 2683ae2..0fdd49e 100755
> --- a/src/lxc-pve-poststop-hook
> +++ b/src/lxc-pve-poststop-hook
> @@ -75,6 +75,12 @@ PVE::LXC::Tools::lxc_hook('post-stop', 'lxc', sub {
>  	open(my $fh, '>', "/var/lib/lxc/$vmid/reboot")
>  	    or die "failed to create reboot trigger file: $!\n";
>  	close($fh);
> +
> +	# activate all volumes of the container in case pending changes added
> +	# a not yet activated volume
> +	my $vollist = PVE::LXC::Config->get_vm_volumes($conf);
> +	PVE::Storage::activate_volumes($storage_cfg, $vollist);
> +
>  	# cause lxc to stop instead of rebooting
>  	exit(1);
>      }
> diff --git a/src/lxc-pve-prestart-hook b/src/lxc-pve-prestart-hook
> index ed25aa4..8823cad 100755
> --- a/src/lxc-pve-prestart-hook
> +++ b/src/lxc-pve-prestart-hook
> @@ -38,11 +38,6 @@ PVE::LXC::Tools::lxc_hook('pre-start', 'lxc', sub {
>  
>      my $storage_cfg = PVE::Storage::config();
>  
> -    my $vollist = PVE::LXC::Config->get_vm_volumes($conf);
> -    my $loopdevlist = PVE::LXC::Config->get_vm_volumes($conf, 'rootfs');
> -
> -    PVE::Storage::activate_volumes($storage_cfg, $vollist);
> -
>      my $rootdir = $vars->{ROOTFS_PATH};
>  
>      # Delete any leftover reboot-trigger file
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 




      reply	other threads:[~2020-08-04 12:09 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-14 17:50 [pve-devel] " Stoiko Ivanov
2020-08-04 12:09 ` Fabian Grünbichler [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=1596542933.0g19smq090.astroid@nora.none \
    --to=f.gruenbichler@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