* [pve-devel] [PATCH zfsonlinux v2 1/3] zfs-initramfs: add patch to avoid activating all LVs at boot
2025-03-07 9:52 [pve-devel] [PATCH storage/zfsonlinux v2 0/3] fix #4997: lvm: avoid autoactivating (new) LVs after boot Friedrich Weber
@ 2025-03-07 9:52 ` Friedrich Weber
2025-03-07 10:57 ` Friedrich Weber
2025-03-07 11:49 ` Fabian Grünbichler
2025-03-07 9:52 ` [pve-devel] [PATCH pve-storage v2 2/3] lvm: create: use multiple lines for lvcreate commandline definition Friedrich Weber
` (2 subsequent siblings)
3 siblings, 2 replies; 11+ messages in thread
From: Friedrich Weber @ 2025-03-07 9:52 UTC (permalink / raw)
To: pve-devel
zfs-initramfs ships an initramfs-tools boot script that
unconditionally activates all LVs on boot. This can cause issues if
the LV resides on a shared LVM VG on top of a shared LUN, in
particular Fibre Channel / directed-attached SAS LUNs, as these are
usually visible at boot time. See bug #4997 [1] for more details.
To avoid this, patch zfs-initramfs such that it performs LVM
autoactivation instead. Thus, it honors the `--setautoactivation n`
flag that is normally used to disable autoactivation during boot for
particular VGs/LVs.
[1] https://bugzilla.proxmox.com/show_bug.cgi?id=4997
Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
---
Notes:
new in v2
I'm also planning on sending the inner patch upstream, well post here
once I do.
...s-use-LVM-autoactivation-for-activat.patch | 49 +++++++++++++++++++
debian/patches/series | 1 +
2 files changed, 50 insertions(+)
create mode 100644 debian/patches/0012-contrib-initramfs-use-LVM-autoactivation-for-activat.patch
diff --git a/debian/patches/0012-contrib-initramfs-use-LVM-autoactivation-for-activat.patch b/debian/patches/0012-contrib-initramfs-use-LVM-autoactivation-for-activat.patch
new file mode 100644
index 0000000..e51096d
--- /dev/null
+++ b/debian/patches/0012-contrib-initramfs-use-LVM-autoactivation-for-activat.patch
@@ -0,0 +1,49 @@
+From 3726c500deffadce1012b5c5ccf19515bb58bdbb Mon Sep 17 00:00:00 2001
+From: Friedrich Weber <f.weber@proxmox.com>
+Date: Thu, 6 Mar 2025 11:44:36 +0100
+Subject: [PATCH] contrib/initramfs: use LVM autoactivation for activating VGs
+
+Currently, the zfs initramfs-tools boot script under local-top calls
+`vgchange -ay`, which unconditionally activates all logical volumes
+(LVs) in all discovered volume groups (VGs). This causes all LVs to be
+active after boot. However, users may prefer to not activate certain
+VGs/LVs on boot. They might normally use the `--setautoactivation n`
+VG/LV flag or the `auto_activation_volume_list` LVM config option to
+achieve this, but since the script unconditionally activates all LVs,
+neither has an effect.
+
+To fix this, call `vgchange -aay` instead. This triggers LVM
+autoactivation, which honors autoactivation settings such as the
+`--setautoactivation` flag. It is also more in line with the LVM
+documentation, which says autoactivation is "meant to be used by
+activation commands that are run automatically by the system" [1].
+
+Note that this change might break misconfigured setups that have ZFS
+on top of an LV for which autoactivation is disabled.
+
+[1] https://gitlab.com/lvmteam/lvm2/-/blob/cff93e4d/conf/example.conf.in#L1579
+
+Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
+---
+ contrib/initramfs/scripts/local-top/zfs | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/contrib/initramfs/scripts/local-top/zfs b/contrib/initramfs/scripts/local-top/zfs
+index 6b80e9f43..fc455077e 100755
+--- a/contrib/initramfs/scripts/local-top/zfs
++++ b/contrib/initramfs/scripts/local-top/zfs
+@@ -41,9 +41,9 @@ activate_vg()
+ return 1
+ fi
+
+- # Detect and activate available volume groups
++ # Detect and auto-activate available volume groups
+ /sbin/lvm vgscan
+- /sbin/lvm vgchange -a y --sysinit
++ /sbin/lvm vgchange -aay --sysinit
+ return $?
+ }
+
+--
+2.39.5
+
diff --git a/debian/patches/series b/debian/patches/series
index 229027f..71e830f 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -9,3 +9,4 @@
0009-arc-stat-summary-guard-access-to-freshly-introduced-.patch
0010-Fix-nfs_truncate_shares-without-etc-exports.d.patch
0011-zpool-status-tighten-bounds-for-noalloc-stat-availab.patch
+0012-contrib-initramfs-use-LVM-autoactivation-for-activat.patch
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [pve-devel] [PATCH zfsonlinux v2 1/3] zfs-initramfs: add patch to avoid activating all LVs at boot
2025-03-07 9:52 ` [pve-devel] [PATCH zfsonlinux v2 1/3] zfs-initramfs: add patch to avoid activating all LVs at boot Friedrich Weber
@ 2025-03-07 10:57 ` Friedrich Weber
2025-03-07 11:49 ` Fabian Grünbichler
1 sibling, 0 replies; 11+ messages in thread
From: Friedrich Weber @ 2025-03-07 10:57 UTC (permalink / raw)
To: pve-devel
On 07/03/2025 10:52, Friedrich Weber wrote:
> [...]
> Notes:
> new in v2
>
> I'm also planning on sending the inner patch upstream, well post here
> once I do.
Opened a PR upstream: https://github.com/openzfs/zfs/pull/17125
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [pve-devel] [PATCH zfsonlinux v2 1/3] zfs-initramfs: add patch to avoid activating all LVs at boot
2025-03-07 9:52 ` [pve-devel] [PATCH zfsonlinux v2 1/3] zfs-initramfs: add patch to avoid activating all LVs at boot Friedrich Weber
2025-03-07 10:57 ` Friedrich Weber
@ 2025-03-07 11:49 ` Fabian Grünbichler
2025-03-07 17:01 ` Friedrich Weber
1 sibling, 1 reply; 11+ messages in thread
From: Fabian Grünbichler @ 2025-03-07 11:49 UTC (permalink / raw)
To: Proxmox VE development discussion, Friedrich Weber
> Friedrich Weber <f.weber@proxmox.com> hat am 07.03.2025 10:52 CET geschrieben:
>
>
> zfs-initramfs ships an initramfs-tools boot script that
> unconditionally activates all LVs on boot. This can cause issues if
> the LV resides on a shared LVM VG on top of a shared LUN, in
> particular Fibre Channel / directed-attached SAS LUNs, as these are
> usually visible at boot time. See bug #4997 [1] for more details.
>
> To avoid this, patch zfs-initramfs such that it performs LVM
> autoactivation instead. Thus, it honors the `--setautoactivation n`
> flag that is normally used to disable autoactivation during boot for
> particular VGs/LVs.
>
> [1] https://bugzilla.proxmox.com/show_bug.cgi?id=4997
>
> Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
Consider this:
Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
also for the purpose of submitting upstream ;)
> ---
>
> Notes:
> new in v2
>
> I'm also planning on sending the inner patch upstream, well post here
> once I do.
>
> ...s-use-LVM-autoactivation-for-activat.patch | 49 +++++++++++++++++++
> debian/patches/series | 1 +
> 2 files changed, 50 insertions(+)
> create mode 100644 debian/patches/0012-contrib-initramfs-use-LVM-autoactivation-for-activat.patch
>
> diff --git a/debian/patches/0012-contrib-initramfs-use-LVM-autoactivation-for-activat.patch b/debian/patches/0012-contrib-initramfs-use-LVM-autoactivation-for-activat.patch
> new file mode 100644
> index 0000000..e51096d
> --- /dev/null
> +++ b/debian/patches/0012-contrib-initramfs-use-LVM-autoactivation-for-activat.patch
> @@ -0,0 +1,49 @@
> +From 3726c500deffadce1012b5c5ccf19515bb58bdbb Mon Sep 17 00:00:00 2001
> +From: Friedrich Weber <f.weber@proxmox.com>
> +Date: Thu, 6 Mar 2025 11:44:36 +0100
> +Subject: [PATCH] contrib/initramfs: use LVM autoactivation for activating VGs
> +
> +Currently, the zfs initramfs-tools boot script under local-top calls
> +`vgchange -ay`, which unconditionally activates all logical volumes
> +(LVs) in all discovered volume groups (VGs). This causes all LVs to be
> +active after boot. However, users may prefer to not activate certain
> +VGs/LVs on boot. They might normally use the `--setautoactivation n`
> +VG/LV flag or the `auto_activation_volume_list` LVM config option to
> +achieve this, but since the script unconditionally activates all LVs,
> +neither has an effect.
> +
> +To fix this, call `vgchange -aay` instead. This triggers LVM
> +autoactivation, which honors autoactivation settings such as the
> +`--setautoactivation` flag. It is also more in line with the LVM
> +documentation, which says autoactivation is "meant to be used by
> +activation commands that are run automatically by the system" [1].
> +
> +Note that this change might break misconfigured setups that have ZFS
> +on top of an LV for which autoactivation is disabled.
> +
> +[1] https://gitlab.com/lvmteam/lvm2/-/blob/cff93e4d/conf/example.conf.in#L1579
> +
> +Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
> +---
> + contrib/initramfs/scripts/local-top/zfs | 4 ++--
> + 1 file changed, 2 insertions(+), 2 deletions(-)
> +
> +diff --git a/contrib/initramfs/scripts/local-top/zfs b/contrib/initramfs/scripts/local-top/zfs
> +index 6b80e9f43..fc455077e 100755
> +--- a/contrib/initramfs/scripts/local-top/zfs
> ++++ b/contrib/initramfs/scripts/local-top/zfs
> +@@ -41,9 +41,9 @@ activate_vg()
> + return 1
> + fi
> +
> +- # Detect and activate available volume groups
> ++ # Detect and auto-activate available volume groups
> + /sbin/lvm vgscan
> +- /sbin/lvm vgchange -a y --sysinit
> ++ /sbin/lvm vgchange -aay --sysinit
> + return $?
> + }
> +
> +--
> +2.39.5
> +
> diff --git a/debian/patches/series b/debian/patches/series
> index 229027f..71e830f 100644
> --- a/debian/patches/series
> +++ b/debian/patches/series
> @@ -9,3 +9,4 @@
> 0009-arc-stat-summary-guard-access-to-freshly-introduced-.patch
> 0010-Fix-nfs_truncate_shares-without-etc-exports.d.patch
> 0011-zpool-status-tighten-bounds-for-noalloc-stat-availab.patch
> +0012-contrib-initramfs-use-LVM-autoactivation-for-activat.patch
> --
> 2.39.5
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pve-devel] [PATCH pve-storage v2 2/3] lvm: create: use multiple lines for lvcreate commandline definition
2025-03-07 9:52 [pve-devel] [PATCH storage/zfsonlinux v2 0/3] fix #4997: lvm: avoid autoactivating (new) LVs after boot Friedrich Weber
2025-03-07 9:52 ` [pve-devel] [PATCH zfsonlinux v2 1/3] zfs-initramfs: add patch to avoid activating all LVs at boot Friedrich Weber
@ 2025-03-07 9:52 ` Friedrich Weber
2025-03-07 9:52 ` [pve-devel] [PATCH pve-storage v2 3/3] fix #4997: lvm: create: disable autoactivation for new logical volumes Friedrich Weber
2025-03-07 12:14 ` [pve-devel] [PATCH storage/zfsonlinux v2 0/3] fix #4997: lvm: avoid autoactivating (new) LVs after boot Fabian Grünbichler
3 siblings, 0 replies; 11+ messages in thread
From: Friedrich Weber @ 2025-03-07 9:52 UTC (permalink / raw)
To: pve-devel
Makes the definition more amenable for future additions.
No functional change intended.
Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
---
Notes:
new in v2
src/PVE/Storage/LVMPlugin.pm | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 38f7fa1..bb54c71 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -344,7 +344,14 @@ sub lvcreate {
$size .= "k"; # default to kilobytes
}
- my $cmd = ['/sbin/lvcreate', '-aly', '-Wy', '--yes', '--size', $size, '--name', $name];
+ my $cmd = [
+ '/sbin/lvcreate',
+ '-aly',
+ '-Wy',
+ '--yes',
+ '--size', $size,
+ '--name', $name,
+ ];
for my $tag (@$tags) {
push @$cmd, '--addtag', $tag;
}
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* [pve-devel] [PATCH pve-storage v2 3/3] fix #4997: lvm: create: disable autoactivation for new logical volumes
2025-03-07 9:52 [pve-devel] [PATCH storage/zfsonlinux v2 0/3] fix #4997: lvm: avoid autoactivating (new) LVs after boot Friedrich Weber
2025-03-07 9:52 ` [pve-devel] [PATCH zfsonlinux v2 1/3] zfs-initramfs: add patch to avoid activating all LVs at boot Friedrich Weber
2025-03-07 9:52 ` [pve-devel] [PATCH pve-storage v2 2/3] lvm: create: use multiple lines for lvcreate commandline definition Friedrich Weber
@ 2025-03-07 9:52 ` Friedrich Weber
2025-03-07 12:14 ` [pve-devel] [PATCH storage/zfsonlinux v2 0/3] fix #4997: lvm: avoid autoactivating (new) LVs after boot Fabian Grünbichler
3 siblings, 0 replies; 11+ messages in thread
From: Friedrich Weber @ 2025-03-07 9:52 UTC (permalink / raw)
To: pve-devel
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.
This is not necessarily problematic for local LVM VGs, but it is
problematic for VGs on top of a shared LUN used by multiple cluster
nodes (accessed via e.g. iSCSI/Fibre Channel/direct-attached SAS).
Concretely, in a cluster with a shared LVM VG where an LV is active on
nodes 1 and 2, deleting the LV on node 1 will not clean up the
device-mapper device on node 2. If an LV with the same name is
recreated later, the leftover device-mapper device will cause
activation of that LV on node 2 to fail with:
> device-mapper: create ioctl on [...] failed: Device or resource busy
Hence, certain combinations of guest removal (and thus LV removals)
and node reboots can cause guest creation or VM live migration (which
both entail LV activation) to fail with the above error message for
certain VMIDs, see bug #4997 for more information [1].
To avoid this issue in the future, disable autoactivation when
creating new LVs using the `--setautoactivation` flag. With this
setting, LVM autoactivation will not activate these LVs, and the
storage stack will take care of activating/deactivating the LV on the
correct node when needed.
This additionally fixes an issue with multipath on FC/SAS-attached
LUNs where LVs would be activated too early after boot when multipath
is not yet available, see [3] for more details and current workaround.
The `--setautoactivation` flag was introduced with LVM 2.03.12 [2],
Bookworm/PVE 8 ships 2.03.16.
Note that the flag is only set for newly created LVs, so LVs created
before this patch can still trigger #4997. To avoid this, users can
manually disable autoactivation for existing LVs or for the entire VG,
in case the VG contains no LVs that users want to autoactivate.
[1] https://bugzilla.proxmox.com/show_bug.cgi?id=4997
[2] https://gitlab.com/lvmteam/lvm2/-/blob/main/WHATS_NEW
[3] https://pve.proxmox.com/mediawiki/index.php?title=Multipath&oldid=12039#FC/SAS-specific_configuration
Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
---
Notes:
new in v2
probably need to wait until PVE 9 for applying this, see cover letter
src/PVE/Storage/LVMPlugin.pm | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index bb54c71..43d7df8 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -351,6 +351,7 @@ sub lvcreate {
'--yes',
'--size', $size,
'--name', $name,
+ '--setautoactivation', 'n',
];
for my $tag (@$tags) {
push @$cmd, '--addtag', $tag;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [pve-devel] [PATCH storage/zfsonlinux v2 0/3] fix #4997: lvm: avoid autoactivating (new) LVs after boot
2025-03-07 9:52 [pve-devel] [PATCH storage/zfsonlinux v2 0/3] fix #4997: lvm: avoid autoactivating (new) LVs after boot Friedrich Weber
` (2 preceding siblings ...)
2025-03-07 9:52 ` [pve-devel] [PATCH pve-storage v2 3/3] fix #4997: lvm: create: disable autoactivation for new logical volumes Friedrich Weber
@ 2025-03-07 12:14 ` Fabian Grünbichler
2025-03-10 14:01 ` Friedrich Weber
3 siblings, 1 reply; 11+ messages in thread
From: Fabian Grünbichler @ 2025-03-07 12:14 UTC (permalink / raw)
To: Proxmox VE development discussion, Friedrich Weber
> Friedrich Weber <f.weber@proxmox.com> hat am 07.03.2025 10:52 CET geschrieben:
> # Summary
>
> With default settings, LVM autoactivates LVs when it sees a new VG, e.g. after
> boot or iSCSI login. In a cluster with guest disks on a shared LVM VG (e.g. on
> top of iSCSI/Fibre Channel (FC)/direct-attached SAS), this can indirectly cause
> guest creation or migration to fail. See bug #4997 [1] and patch #3 for
> details.
>
> The goal of this series is to avoid autoactivating LVs that hold guest disks in
> order to fix #4997. With this series applied, (newly created) LVs should only
> be active on the node that hosts the corresponding guest.
>
> # Difference to v1
>
> v1 [0] went with an approach that creates new LVs with the "activation skip"
> flag set, which means lvchange will only activate the LV if the `-K` flag is
> passed. This approach requires a two-step patch apply process to not break
> mixed-version clusters: With PVE 8.4, we would add a change that passes the
> `-K` flag when activating, and only with PVE 9 we would also create new LVs
> with the "activation skip" flag. This approach works but is relatively complex.
>
> # This v2 approach
>
> This v2 goes a different route: Instead of "activation skip" flag, we use the
> the `--setautoactivation n` flag for LVs. This flag is available in Debian's
> LVM since Bookworm and only concerns autoactivation, not normal activation as
> performed by our LVM plugin. Hence, it is enough to create new LVs with
> `--setautoactivation n`, we don't require any two-step patch apply process. We
> may still need to wait for PVE 9 to apply this though (see next section).
> Considering this, the advantage over v1 may appear quite small, though I still
> find v2's approach preferable because the `--setautoactivation` flag seems less
> invasive than the activation skip flag.
thanks for the clear description and analysis! :)
> # Mixed 7/8 cluster
>
> Unfortunately, we need to consider the mixed-version cluster between PVE 7 and
> PVE 8 because PVE 7/Bullseye's LVM does not know `--setautoactivation`. A user
> upgrading from PVE 7 will temporarily have a mixed 7/8 cluster. Once this
> series is applied, the PVE 8 nodes will create new LVs with
> `--setautoactivation n`, which the PVE 7 nodes do not know. In my tests, the
> PVE 7 nodes can read/interact with such LVs just fine, *but*: As soon as a PVE
> 7 node creates a new (unrelated) LV, the `--setautoactivation n` flag is reset
> to default `y` on *all* LVs of the VG. I presume this is because creating a new
> LV rewrites metadata, and the PVE 7 LVM doesn't write out the
> `--setautoactivation n` flag. I imagine (have not tested) this will cause
> problems on a mixed cluster.
>
> Hence, it may be safer to hold off on applying patches #2/#3 until PVE 9,
> because then we can be sure all nodes run at least PVE 8.
wow - I wonder what other similar timebombs are lurking there??
e.g., for the changelog for bullseye -> bookworm:
2.03.15: "Increase some hash table size to better support large device sets." (hopefully those are in memory?)
2.03.14: various VDO things (not used by us at least)
2.03.12: "Allow attaching cache to thin data volume." and other thin-related changes (not relevant for shared thankfully), auto activation, more VDO things
bookworm -> trixie:
2.03.28: various things using radix_trees now (hopefully in memory?)
2.03.25: same
2.03.24: "Support external origin between different thin-pool." & "Support creation of thin-pool with VDO use for its data volume" (local only thankfully),
2.03.23: "Set the first lv_attr flag for raid integrity images to i or I." (not used by us, maybe in memory only)
2.03.22: "Support parsing of vdo geometry format version 4.", "Fix parsing of VDO metadata.", "Allow snapshots of raid+integrity LV." (all not applicable to regular PVE setups)
2.03.18: "Add support for writecache metadata_only and pause_writeback settings." (?)
so at least nothing that jumps out as definitely problematic for our common setups..
> # Interaction with zfs-initramfs
>
> One complication is that zfs-initramfs ships an initramfs-tools script that
> unconditionally activates *all* VGs that are visible at boot time, see [2].
> This is the case for local VGs, but also for shared VGs on FC/SAS. Patch #2 of
> this series fixes this by making the script perform autoactivation instead,
> which honors the `--setautoactivation` flag. The patch is necessary to fix
> #4997 on FC/SAS-attached LUNs too.
>
> # Bonus fix for FC/SAS multipath+LVM issue
>
> As it turns out, this series seems to additionally fix an issue on hosts with
> LVM on FC/SAS-attached LUNs *with multipath* where LVM would report "Device
> mismatch detected" warnings because the LVs are activated too early in the boot
> process before multipath is available. Our current suggested workaround is to
> install multipath-tools-boot [2]. With this series applied, this shouldn't be
> necessary anymore, as (newly created) LVs are not auto-activated after boot.
>
> # LVM-thick/LVM-thin
>
> Note that this change affects all LVs on LVM-thick, not just ones on shared
> storage. As a result, also on single-node hosts, local guest disk LVs on
> LVM-thick will not be automatically active after boot anymore (after applying
> all patches of this series). Guest disk LVs on LVM-thin will still be
> auto-activated, but since LVM-thin storage is necessarily local, we don't run
> into #4997 here.
we could check the shared property, but I don't think having them not
auto-activated hurts as long as it is documented..
>
> # Transition to LVs with `--setautoactivation n`
>
> Both v1 and v2 approaches only take effect for new LVs, so we should probably
> also have pve8to9 check for guest disks on (shared?) LVM that have
> autoactivation enabled, and suggest to the user to manually disable
> autoactivation on the LVs, or even the entire VG if it holds only PVE-managed
> LVs.
if we want to wait for PVE 9 anyway to start enabling (disabling? ;)) it, then
the upgrade script would be a nice place to tell users to fix up their volumes?
OTOH, setting the flag automatically starting with PVE 9 also for existing
volumes should have no downsides, we need to document anyway that the behaviour
there changed (so that people that rely on them becoming auto-activated on boot
can adapt whatever is relying on that).. or we could provide a script that does
it post-upgrade..
> We could implement something on top to make the transition smoother, some ideas:
>
> - When activating existing LVs, check the auto activation flag, and if auto
> activation is still enabled, disable it.
the only question is whether we want to "pay" for that on each activate_volume?
> - When creating a new VG via the LVM storage plugin or the /disks/lvm API
> endpoints, disable autoactivation for the whole VG. But this may become
> confusing as then we'd deal with the flag on both LVs and VGs. Also, especially
> for FC/SAS-attached LUNs, users may not use the GUI/API and instead create the
> VG via the command line. We could adjust our guides to use `vgcreate
> --setautoactivation n` [4], but not all users may follow these guides.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [pve-devel] [PATCH storage/zfsonlinux v2 0/3] fix #4997: lvm: avoid autoactivating (new) LVs after boot
2025-03-07 12:14 ` [pve-devel] [PATCH storage/zfsonlinux v2 0/3] fix #4997: lvm: avoid autoactivating (new) LVs after boot Fabian Grünbichler
@ 2025-03-10 14:01 ` Friedrich Weber
2025-03-11 10:40 ` Fabian Grünbichler
0 siblings, 1 reply; 11+ messages in thread
From: Friedrich Weber @ 2025-03-10 14:01 UTC (permalink / raw)
To: Fabian Grünbichler, Proxmox VE development discussion
On 07/03/2025 13:14, Fabian Grünbichler wrote:
> [...]
>> # Mixed 7/8 cluster
>>
>> Unfortunately, we need to consider the mixed-version cluster between PVE 7 and
>> PVE 8 because PVE 7/Bullseye's LVM does not know `--setautoactivation`. A user
>> upgrading from PVE 7 will temporarily have a mixed 7/8 cluster. Once this
>> series is applied, the PVE 8 nodes will create new LVs with
>> `--setautoactivation n`, which the PVE 7 nodes do not know. In my tests, the
>> PVE 7 nodes can read/interact with such LVs just fine, *but*: As soon as a PVE
>> 7 node creates a new (unrelated) LV, the `--setautoactivation n` flag is reset
>> to default `y` on *all* LVs of the VG. I presume this is because creating a new
>> LV rewrites metadata, and the PVE 7 LVM doesn't write out the
>> `--setautoactivation n` flag. I imagine (have not tested) this will cause
>> problems on a mixed cluster.
>>
>> Hence, it may be safer to hold off on applying patches #2/#3 until PVE 9,
>> because then we can be sure all nodes run at least PVE 8.
>
> wow - I wonder what other similar timebombs are lurking there??
Huh, good question, ...
>
> e.g., for the changelog for bullseye -> bookworm:
>
> 2.03.15: "Increase some hash table size to better support large device sets." (hopefully those are in memory?)
> 2.03.14: various VDO things (not used by us at least)
> 2.03.12: "Allow attaching cache to thin data volume." and other thin-related changes (not relevant for shared thankfully), auto activation, more VDO things
>
> bookworm -> trixie:
>
> 2.03.28: various things using radix_trees now (hopefully in memory?)
> 2.03.25: same
> 2.03.24: "Support external origin between different thin-pool." & "Support creation of thin-pool with VDO use for its data volume" (local only thankfully),
> 2.03.23: "Set the first lv_attr flag for raid integrity images to i or I." (not used by us, maybe in memory only)
> 2.03.22: "Support parsing of vdo geometry format version 4.", "Fix parsing of VDO metadata.", "Allow snapshots of raid+integrity LV." (all not applicable to regular PVE setups)
> 2.03.18: "Add support for writecache metadata_only and pause_writeback settings." (?)
This is probably referring to new lvmcache options [1], shouldn't be
relevant for our default setup I guess.
>
> so at least nothing that jumps out as definitely problematic for our common setups..
... thanks for taking a look. I didn't see anything problematic either.
>> # Interaction with zfs-initramfs
>>
>> One complication is that zfs-initramfs ships an initramfs-tools script that
>> unconditionally activates *all* VGs that are visible at boot time, see [2].
>> This is the case for local VGs, but also for shared VGs on FC/SAS. Patch #2 of
>> this series fixes this by making the script perform autoactivation instead,
>> which honors the `--setautoactivation` flag. The patch is necessary to fix
>> #4997 on FC/SAS-attached LUNs too.
>>
>> # Bonus fix for FC/SAS multipath+LVM issue
>>
>> As it turns out, this series seems to additionally fix an issue on hosts with
>> LVM on FC/SAS-attached LUNs *with multipath* where LVM would report "Device
>> mismatch detected" warnings because the LVs are activated too early in the boot
>> process before multipath is available. Our current suggested workaround is to
>> install multipath-tools-boot [2]. With this series applied, this shouldn't be
>> necessary anymore, as (newly created) LVs are not auto-activated after boot.
>>
>> # LVM-thick/LVM-thin
>>
>> Note that this change affects all LVs on LVM-thick, not just ones on shared
>> storage. As a result, also on single-node hosts, local guest disk LVs on
>> LVM-thick will not be automatically active after boot anymore (after applying
>> all patches of this series). Guest disk LVs on LVM-thin will still be
>> auto-activated, but since LVM-thin storage is necessarily local, we don't run
>> into #4997 here.
>
> we could check the shared property, but I don't think having them not
> auto-activated hurts as long as it is documented..
This is referring to LVs on *local* LVM-*thick* storage, right? In that
case, I'd agree that not having them autoactivated either is okay
(cleaner even).
The patch series currently doesn't touch the LvmThinPlugin at all, so
all LVM-*thin* LVs will still be auto-activated at boot. We could also
patch LvmThinPlugin to create new thin LVs with `--setautoactivation n`
-- though it wouldn't give us much, except consistency with LVM-thick.
>> # Transition to LVs with `--setautoactivation n`
>>
>> Both v1 and v2 approaches only take effect for new LVs, so we should probably
>> also have pve8to9 check for guest disks on (shared?) LVM that have
>> autoactivation enabled, and suggest to the user to manually disable
>> autoactivation on the LVs, or even the entire VG if it holds only PVE-managed
>> LVs.
>
> if we want to wait for PVE 9 anyway to start enabling (disabling? ;)) it, then
> the upgrade script would be a nice place to tell users to fix up their volumes?
The upgrade script being pve8to9, right? I'm just not sure yet what to
suggest: `lvchange --setautoactivation n` on each LV, or simply
`vgchange --setautoactivation n` on the whole shared VG (provided it
only contains PVE-managed LVs).
> OTOH, setting the flag automatically starting with PVE 9 also for existing
> volumes should have no downsides, [...]
Hmm, but how would be do that automatically?
> we need to document anyway that the behaviour
> there changed (so that people that rely on them becoming auto-activated on boot
> can adapt whatever is relying on that).. or we could provide a script that does
> it post-upgrade..
Yes, an extra script to run after the upgrade might be an option. Though
we'd also need to decide whether to deactivate on each individual LV, or
the complete VG (then we'd just assume that there no other
non-PVE-managed LVs in the VG that the user wants autoactivated).
>
>> We could implement something on top to make the transition smoother, some ideas:
>>
>> - When activating existing LVs, check the auto activation flag, and if auto
>> activation is still enabled, disable it.
>
> the only question is whether we want to "pay" for that on each activate_volume?
Good question. It does seem a little extreme, also considering that once
all existing LVs have autoactivation disabled, all new LVs will have the
flag disabled as well and the check becomes obsolete.
It just occurred to me that we could also pass `--setautoactivation n`
to `lvchange -ay` in `activate_volume`, but a test shows that this
triggers a metadata update on *each activate_volume*, which sounds like
a bad idea.
>
>> - When creating a new VG via the LVM storage plugin or the /disks/lvm API
>> endpoints, disable autoactivation for the whole VG. But this may become
>> confusing as then we'd deal with the flag on both LVs and VGs. Also, especially
>> for FC/SAS-attached LUNs, users may not use the GUI/API and instead create the
>> VG via the command line. We could adjust our guides to use `vgcreate
>> --setautoactivation n` [4], but not all users may follow these guides.
[1] https://manpages.debian.org/trixie/lvm2/lvmcache.7.en.html#metadata_only
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [pve-devel] [PATCH storage/zfsonlinux v2 0/3] fix #4997: lvm: avoid autoactivating (new) LVs after boot
2025-03-10 14:01 ` Friedrich Weber
@ 2025-03-11 10:40 ` Fabian Grünbichler
2025-03-12 8:39 ` Friedrich Weber
0 siblings, 1 reply; 11+ messages in thread
From: Fabian Grünbichler @ 2025-03-11 10:40 UTC (permalink / raw)
To: Friedrich Weber, Proxmox VE development discussion
On March 10, 2025 3:01 pm, Friedrich Weber wrote:
> On 07/03/2025 13:14, Fabian Grünbichler wrote:
>>> # LVM-thick/LVM-thin
>>>
>>> Note that this change affects all LVs on LVM-thick, not just ones on shared
>>> storage. As a result, also on single-node hosts, local guest disk LVs on
>>> LVM-thick will not be automatically active after boot anymore (after applying
>>> all patches of this series). Guest disk LVs on LVM-thin will still be
>>> auto-activated, but since LVM-thin storage is necessarily local, we don't run
>>> into #4997 here.
>>
>> we could check the shared property, but I don't think having them not
>> auto-activated hurts as long as it is documented..
>
> This is referring to LVs on *local* LVM-*thick* storage, right? In that
> case, I'd agree that not having them autoactivated either is okay
> (cleaner even).
yes
> The patch series currently doesn't touch the LvmThinPlugin at all, so
> all LVM-*thin* LVs will still be auto-activated at boot. We could also
> patch LvmThinPlugin to create new thin LVs with `--setautoactivation n`
> -- though it wouldn't give us much, except consistency with LVM-thick.
well, if you have many volumes not activating them automatically might
also save some time on boot ;) but yeah, shouldn't cause any issues.
>>> # Transition to LVs with `--setautoactivation n`
>>>
>>> Both v1 and v2 approaches only take effect for new LVs, so we should probably
>>> also have pve8to9 check for guest disks on (shared?) LVM that have
>>> autoactivation enabled, and suggest to the user to manually disable
>>> autoactivation on the LVs, or even the entire VG if it holds only PVE-managed
>>> LVs.
>>
>> if we want to wait for PVE 9 anyway to start enabling (disabling? ;)) it, then
>> the upgrade script would be a nice place to tell users to fix up their volumes?
>
> The upgrade script being pve8to9, right? I'm just not sure yet what to
> suggest: `lvchange --setautoactivation n` on each LV, or simply
> `vgchange --setautoactivation n` on the whole shared VG (provided it
> only contains PVE-managed LVs).
yes.
>
>> OTOH, setting the flag automatically starting with PVE 9 also for existing
>> volumes should have no downsides, [...]
>
> Hmm, but how would be do that automatically?
e.g., once on upgrading in postinst, or in activate_storage if we find a
cheap way to skip doing it over and over ;)
>
>> we need to document anyway that the behaviour
>> there changed (so that people that rely on them becoming auto-activated on boot
>> can adapt whatever is relying on that).. or we could provide a script that does
>> it post-upgrade..
>
> Yes, an extra script to run after the upgrade might be an option. Though
> we'd also need to decide whether to deactivate on each individual LV, or
> the complete VG (then we'd just assume that there no other
> non-PVE-managed LVs in the VG that the user wants autoactivated).
I think doing it on each volume managed by PVE is the safer and more
consistent option..
>>> We could implement something on top to make the transition smoother, some ideas:
>>>
>>> - When activating existing LVs, check the auto activation flag, and if auto
>>> activation is still enabled, disable it.
>>
>> the only question is whether we want to "pay" for that on each activate_volume?
>
> Good question. It does seem a little extreme, also considering that once
> all existing LVs have autoactivation disabled, all new LVs will have the
> flag disabled as well and the check becomes obsolete.
>
> It just occurred to me that we could also pass `--setautoactivation n`
> to `lvchange -ay` in `activate_volume`, but a test shows that this
> triggers a metadata update on *each activate_volume*, which sounds like
> a bad idea.
yeah that doesn't sound too good ;)
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [pve-devel] [PATCH storage/zfsonlinux v2 0/3] fix #4997: lvm: avoid autoactivating (new) LVs after boot
2025-03-11 10:40 ` Fabian Grünbichler
@ 2025-03-12 8:39 ` Friedrich Weber
0 siblings, 0 replies; 11+ messages in thread
From: Friedrich Weber @ 2025-03-12 8:39 UTC (permalink / raw)
To: Fabian Grünbichler, Proxmox VE development discussion
On 11/03/2025 11:40, Fabian Grünbichler wrote:
> On March 10, 2025 3:01 pm, Friedrich Weber wrote:
>> On 07/03/2025 13:14, Fabian Grünbichler wrote:
>>>> # LVM-thick/LVM-thin
>>>>
>>>> Note that this change affects all LVs on LVM-thick, not just ones on shared
>>>> storage. As a result, also on single-node hosts, local guest disk LVs on
>>>> LVM-thick will not be automatically active after boot anymore (after applying
>>>> all patches of this series). Guest disk LVs on LVM-thin will still be
>>>> auto-activated, but since LVM-thin storage is necessarily local, we don't run
>>>> into #4997 here.
>>>
>>> we could check the shared property, but I don't think having them not
>>> auto-activated hurts as long as it is documented..
>>
>> This is referring to LVs on *local* LVM-*thick* storage, right? In that
>> case, I'd agree that not having them autoactivated either is okay
>> (cleaner even).
>
> yes
>
>> The patch series currently doesn't touch the LvmThinPlugin at all, so
>> all LVM-*thin* LVs will still be auto-activated at boot. We could also
>> patch LvmThinPlugin to create new thin LVs with `--setautoactivation n`
>> -- though it wouldn't give us much, except consistency with LVM-thick.
>
> well, if you have many volumes not activating them automatically might
> also save some time on boot ;) but yeah, shouldn't cause any issues.
Yeah, then I'll limit this change to the LVMPlugin and keep the
LvmThinPlugin unchanged.
>>>> # Transition to LVs with `--setautoactivation n`
>>>>
>>>> Both v1 and v2 approaches only take effect for new LVs, so we should probably
>>>> also have pve8to9 check for guest disks on (shared?) LVM that have
>>>> autoactivation enabled, and suggest to the user to manually disable
>>>> autoactivation on the LVs, or even the entire VG if it holds only PVE-managed
>>>> LVs.
>>>
>>> if we want to wait for PVE 9 anyway to start enabling (disabling? ;)) it, then
>>> the upgrade script would be a nice place to tell users to fix up their volumes?
>>
>> The upgrade script being pve8to9, right? I'm just not sure yet what to
>> suggest: `lvchange --setautoactivation n` on each LV, or simply
>> `vgchange --setautoactivation n` on the whole shared VG (provided it
>> only contains PVE-managed LVs).
>
> yes.
>
>>
>>> OTOH, setting the flag automatically starting with PVE 9 also for existing
>>> volumes should have no downsides, [...]
>>
>> Hmm, but how would be do that automatically?
>
> e.g., once on upgrading in postinst, or in activate_storage if we find a
> cheap way to skip doing it over and over ;)
>
>>
>>> we need to document anyway that the behaviour
>>> there changed (so that people that rely on them becoming auto-activated on boot
>>> can adapt whatever is relying on that).. or we could provide a script that does
>>> it post-upgrade..
>>
>> Yes, an extra script to run after the upgrade might be an option. Though
>> we'd also need to decide whether to deactivate on each individual LV, or
>> the complete VG (then we'd just assume that there no other
>> non-PVE-managed LVs in the VG that the user wants autoactivated).
>
> I think doing it on each volume managed by PVE is the safer and more
> consistent option..
Sounds good!
Discussed the next steps with Fabian off-list, the summary:
- I'll wait a few days if zfsonlinux patch 1 gets applied upstream so I
can send a proper cherry-pick. We can apply that one before PVE 9, as it
shouldn't break any reasonable setup (= setup that doesn't have ZFS on
an LVM LV with autoactivation disabled), and it allows the manual
`vgchange --setautoactivation n` workaround to work for FC/SAS LUNs
before PVE 9.
- we keep something like patch 2+3 which pass `--setautoactivation n` to
lvcreate in LVMPlugin's `alloc_image`. This can only be applied for PVE
9 (see "Mixed 7/8 cluster" in cover letter). It solves the
auto-activation problem for new volumes.
- to solve the auto-activation problem for existing volumes, we
- ship a script (a separate script or even a subcommand of pve8to9?)
that disables autoactivation for all PVE-managed LVs on shared VGs,
after asking for user confirmation. The user has to run this script
manually. The script probably needs to take care to cluster-lock the
storage, as disabling autoactivation triggers a metadata update.
- add a check to pve8to9 for PVE-managed LVs with autoactivation
enabled (on shared VGs) which suggests to run the script from the
previous point
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 11+ messages in thread