all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Michael Köppl" <m.koeppl@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Friedrich Weber <f.weber@proxmox.com>
Subject: Re: [pve-devel] [PATCH pve-storage v3 3/3] lvmthin: disable autoactivation for new logical volumes
Date: Tue, 10 Jun 2025 17:00:36 +0200	[thread overview]
Message-ID: <c8ce5e46-bf86-44bf-91ed-c529d935ceb4@proxmox.com> (raw)
In-Reply-To: <20250429113646.25738-4-f.weber@proxmox.com>

On 4/29/25 13:36, Friedrich Weber wrote:
> When discovering a new volume group (VG), for example on boot, LVM
> triggers autoactivation. With the default settings, this activates all
> logical volumes (LVs) in the VG. Activating an LV creates a
> device-mapper device and a block device under /dev/mapper.
> 
> Autoactivation is problematic for shared LVM storages, see #4997 [1].
> For the inherently local LVM-thin storage it is less problematic, but
> it still makes sense to avoid unnecessarily activating LVs and thus
> making them visible on the host at boot.
> 
> Hence, disable autoactivation after creating new LVs. As lvcreate
> doesn't accept the --setautoactivation flag for thin LVs, this is done
> with an additional lvchange command. With this setting, LVM
> autoactivation will not activate these LVs, and the storage stack will
> take care of activating/deactivating LVs when needed.
> 
> [1] https://bugzilla.proxmox.com/show_bug.cgi?id=4997
> 
> Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
> ---
> 
> Notes:
>     - would be great to get your opinion on whether we should consider
>       LVM-thin storages in this series or not.
>     
>     - passing --setautoactivation n to lvcreate for a thin volume says:
>     
>         Option --setautoactivation is unsupported with thins.
>     
>       But lvchange --setautoactivation seems to work on thin LVs, so the
>       fact that lvcreate doesn't accept it may be a bug. I reported it
>       upstream [1].
>     
>     new in v3
>     
>     [1] https://gitlab.com/lvmteam/lvm2/-/issues/32

Since the upstream issue has not been addressed yet and the change to
LVM-thin does, AFAICT, not mitigate problems like in #4997 (or am I
missing something here?), but is mostly done to streamline behavior,
could the changes for LVM-thin be held back until it's clear that
lvcreate not supporting --setautoactivation for LVM-thin is not on purpose?

> 
>  src/PVE/Storage/LvmThinPlugin.pm | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Storage/LvmThinPlugin.pm b/src/PVE/Storage/LvmThinPlugin.pm
> index 49a4dcb..3f75ba1 100644
> --- a/src/PVE/Storage/LvmThinPlugin.pm
> +++ b/src/PVE/Storage/LvmThinPlugin.pm
> @@ -82,6 +82,19 @@ sub filesystem_path {
>      return wantarray ? ($path, $vmid, $vtype) : $path;
>  }
>  
> +# lvcreate refuses --setautoactivation for thin volumes, so set it via lvchange
> +my $set_lv_autoactivation = sub {
> +    my ($vg, $lv, $autoactivation) = @_;
> +
> +    my $cmd = [
> +	'/sbin/lvchange',
> +	'--setautoactivation', $autoactivation ? 'y' : 'n',
> +	"$vg/$lv"
> +    ];
> +    eval { run_command($cmd); };
> +    warn "could not set autoactivation: $@" if $@;
> +};
> +
>  sub alloc_image {
>      my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
>  
> @@ -103,6 +116,7 @@ sub alloc_image {
>  	       '--thinpool', "$vg/$scfg->{thinpool}" ];
>  
>      run_command($cmd, errmsg => "lvcreate '$vg/$name' error");
> +    $set_lv_autoactivation->($vg, $name, 0);
>  
>      return $name;
>  }
> @@ -283,6 +297,7 @@ sub clone_image {
>  
>      my $cmd = ['/sbin/lvcreate', '-n', $name, '-prw', '-kn', '-s', $lv];
>      run_command($cmd, errmsg => "clone image '$lv' error");
> +    $set_lv_autoactivation->($vg, $name, 0);
>  
>      return $name;
>  }
> @@ -332,7 +347,7 @@ sub volume_snapshot {
>  
>      my $cmd = ['/sbin/lvcreate', '-n', $snapvol, '-pr', '-s', "$vg/$volname"];
>      run_command($cmd, errmsg => "lvcreate snapshot '$vg/$snapvol' error");
> -
> +    # disabling autoactivation not needed, as -s defaults to --setautoactivationskip y
>  }
>  
>  sub volume_snapshot_rollback {
> @@ -346,6 +361,7 @@ sub volume_snapshot_rollback {
>  
>      $cmd = ['/sbin/lvcreate', '-kn', '-n', $volname, '-s', "$vg/$snapvol"];
>      run_command($cmd, errmsg => "lvm rollback '$vg/$snapvol' error");
> +    $set_lv_autoactivation->($vg, $volname, 0);
>  }
>  
>  sub volume_snapshot_delete {



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


  reply	other threads:[~2025-06-10 15:00 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-29 11:36 [pve-devel] [RFC storage/manager v3 0/6] fix #4997: lvm, lvm-thin: avoid autoactivating LVs Friedrich Weber
2025-04-29 11:36 ` [pve-devel] [PATCH pve-storage v3 1/3] lvm: create: use multiple lines for lvcreate command line Friedrich Weber
2025-04-29 11:36 ` [pve-devel] [PATCH pve-storage v3 2/3] fix #4997: lvm: create: disable autoactivation for new logical volumes Friedrich Weber
2025-04-29 11:36 ` [pve-devel] [PATCH pve-storage v3 3/3] lvmthin: " Friedrich Weber
2025-06-10 15:00   ` Michael Köppl [this message]
2025-06-23  9:25     ` Friedrich Weber
2025-06-27  8:14       ` Fabian Grünbichler
2025-04-29 11:36 ` [pve-devel] [PATCH pve-manager stable-8 v3 1/3] cli: create pve8to9 script as a copy of pve7to8 Friedrich Weber
2025-04-29 11:36 ` [pve-devel] [PATCH pve-manager stable-8 v3 2/3] pve8to9: move checklist to dedicated subcommand Friedrich Weber
2025-04-29 11:36 ` [pve-devel] [PATCH pve-manager stable-8 v3 3/3] pve8to9: detect and (if requested) disable LVM autoactivation Friedrich Weber
2025-06-10 14:25   ` Michael Köppl
2025-06-23  9:25     ` Friedrich Weber
2025-06-10 15:03 ` [pve-devel] [RFC storage/manager v3 0/6] fix #4997: lvm, lvm-thin: avoid autoactivating LVs Michael Köppl
2025-06-23  9:26   ` Friedrich Weber

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=c8ce5e46-bf86-44bf-91ed-c529d935ceb4@proxmox.com \
    --to=m.koeppl@proxmox.com \
    --cc=f.weber@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