[pve-devel] [PATCH storage 0/2] fix #4997: lvm: avoid autoactivating (new) LVs after boot

[pve-devel] [PATCH storage 0/2] fix #4997: lvm: avoid autoactivating (new) LVs after boot

[Friedrich Weber]

By default, LVM autoactivates LVs after boot. In a cluster with VM disks on a
shared LVM VG (e.g. on top of iSCSI), this can indirectly cause guest creation
or VM live-migration to fail. See bug #4997 [1] and patch #2 for details.

The goal of this series is to avoid autoactivating LVs after boot. Fabian
suggested to use the "activation skip" flag for LVs. LVs with that flag can
only be activated if the `-K` flag is passed during activation (`-K` is not
passed for autoactivation after boot).

With patch #1, the LVM plugin passes the `-K` flag to activation commands. If
the LV does not have the "activation skip" flag set, this should not have any
effect, so it should be safe to apply this patch in the near future. It does
not yet fix #4997, though.

With patch #2, the LVM plugin sets the "activation skip" flag for newly created
LVs. As this can be considered a breaking change, it may make sense to only
apply it close to the PVE 9 release. If patch #1 has been available in the last
PVE 8 minor release, this should ensure a smooth upgrade even if a cluster is
temporarily mixed between PVE 8.x and 9 (PVE 8.x will be able to activate LVs
created by PVE 9 with "activation skip"). This will fix #4997 for newly created
LVs.

Some points to discuss:

* Fabian and I discussed whether it may be better to pass `-K` and set the
  "activation skip" flag only for LVs on a *shared* LVM storage. But this may
  cause issues for users that incorrectly mark an LVM storage as shared, create a
  bunch of LVs (with "activation skip" flag), then unset the "shared" flag, and
  won't be able to activate LVs afterwards (`lvchange -ay` without `-K` on an LV
  with "activation skip" is a noop). What do you think?

* Even with patch #1 and #2 applied, users can still run into #4997 for LVs
  that were created before #2, so without the "activation skip" flag. So it might
  be good to include a note in the 8->9 release notes and/or a warning in the
  pve8to9 helper and/or ship a script that automatically sets the flag for all
  existing (PVE-owned) LVs.

[1] https://bugzilla.proxmox.com/show_bug.cgi?id=4997

storage:

Friedrich Weber (2):
  lvm: ignore "activation skip" LV flag during LV activation
  fix #4997: lvm: set "activation skip" flag for newly created LVs

 src/PVE/Storage/LVMPlugin.pm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)


Summary over all repositories:
  1 files changed, 3 insertions(+), 0 deletions(-)

-- 
Generated by git-murpp 0.5.0

[pve-devel] [PATCH storage 1/2] lvm: ignore "activation skip" LV flag during LV activation

[Friedrich Weber]

LVs with the "activation skip" flag (can be set via `lvchange -ky`)
are only activated if `-K` is passed to the activation command. In the
next major release, we intend to set the "activation skip" flag for
newly created LVs. The goal is to prevent LVs from being
auto-activated after boot, and thus fix bug #4997.

In preparation for this future change, adjust activation commands
issued by the LVM plugin to include the `-K` flag. For LVs without the
"activation skip" flag, passing the `-K` flag during activation has no
effect, so it should be harmless to always provide it.

[1] https://bugzilla.proxmox.com/show_bug.cgi?id=4997

Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
---
 src/PVE/Storage/LVMPlugin.pm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 4b951e7..66b42de 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -342,7 +342,7 @@ sub lvcreate {
 	$size .= "k"; # default to kilobytes
     }
 
-    my $cmd = ['/sbin/lvcreate', '-aly', '-Wy', '--yes', '--size', $size, '--name', $name];
+    my $cmd = ['/sbin/lvcreate', '-aly', '-K', '-Wy', '--yes', '--size', $size, '--name', $name];
     for my $tag (@$tags) {
 	push @$cmd, '--addtag', $tag;
     }
@@ -422,7 +422,7 @@ sub free_image {
 	print "successfully removed volume $volname ($vg/del-$volname)\n";
     };
 
-    my $cmd = ['/sbin/lvchange', '-aly', "$vg/$volname"];
+    my $cmd = ['/sbin/lvchange', '-aly', '-K', "$vg/$volname"];
     run_command($cmd, errmsg => "can't activate LV '$vg/$volname' to zero-out its data");
     $cmd = ['/sbin/lvchange', '--refresh', "$vg/$volname"];
     run_command($cmd, errmsg => "can't refresh LV '$vg/$volname' to zero-out its data");
@@ -536,7 +536,7 @@ sub activate_volume {
 
     my $lvm_activate_mode = 'ey';
 
-    my $cmd = ['/sbin/lvchange', "-a$lvm_activate_mode", $path];
+    my $cmd = ['/sbin/lvchange', "-a$lvm_activate_mode", '-K', $path];
     run_command($cmd, errmsg => "can't activate LV '$path'");
     $cmd = ['/sbin/lvchange', '--refresh', $path];
     run_command($cmd, errmsg => "can't refresh LV '$path' for activation");
-- 
2.39.2

Re: [pve-devel] [PATCH storage 1/2] lvm: ignore "activation skip" LV flag during LV activation

[Fiona Ebner]

Am 11.01.24 um 16:03 schrieb Friedrich Weber:
> LVs with the "activation skip" flag (can be set via `lvchange -ky`)
> are only activated if `-K` is passed to the activation command. In the
> next major release, we intend to set the "activation skip" flag for
> newly created LVs. The goal is to prevent LVs from being
> auto-activated after boot, and thus fix bug #4997.
> 
> In preparation for this future change, adjust activation commands
> issued by the LVM plugin to include the `-K` flag. For LVs without the
> "activation skip" flag, passing the `-K` flag during activation has no
> effect, so it should be harmless to always provide it.
> 
> [1] https://bugzilla.proxmox.com/show_bug.cgi?id=4997
> 
> Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> Signed-off-by: Friedrich Weber <f.weber@proxmox.com>

Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>

> ---
>  src/PVE/Storage/LVMPlugin.pm | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
> index 4b951e7..66b42de 100644
> --- a/src/PVE/Storage/LVMPlugin.pm
> +++ b/src/PVE/Storage/LVMPlugin.pm
> @@ -342,7 +342,7 @@ sub lvcreate {
>  	$size .= "k"; # default to kilobytes
>      }
>  
> -    my $cmd = ['/sbin/lvcreate', '-aly', '-Wy', '--yes', '--size', $size, '--name', $name];
> +    my $cmd = ['/sbin/lvcreate', '-aly', '-K', '-Wy', '--yes', '--size', $size, '--name', $name];

Style nit: line too long

>      for my $tag (@$tags) {
>  	push @$cmd, '--addtag', $tag;
>      }

[pve-devel] [PATCH storage 2/2] fix #4997: lvm: set "activation skip" flag for newly created LVs

[Friedrich Weber]

Activating an LV creates a device-mapper device. In a cluster with a
shared LVM VG (e.g. on top of iSCSI) 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

By default, LVM autoactivates all discovered LVs after boot, thus
creating device-mapper device for all discovered LVs. As a result,
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, see [1].

To avoid this issue in the future, adjust the LVM plugin to create new
LVs with the "activation skip" flag. LVs with that flag are not
activated unless `-K` is passed to the activation command. Consequently,
new LVs will not be autoactivated after boot anymore, and removing LVs
will not leave behind device-mapper devices on other nodes anymore.

The LVM plugin is still able to activate LVs, as it already passes
`-K` to all activation commands.

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 set the "activation skip" flag on existing LVs.

[1] https://bugzilla.proxmox.com/show_bug.cgi?id=4997

Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
---

Notes:
    Should only be applied close to the next major release, see cover
    letter.

 src/PVE/Storage/LVMPlugin.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 66b42de..191e8d3 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -342,7 +342,7 @@ sub lvcreate {
 	$size .= "k"; # default to kilobytes
     }
 
-    my $cmd = ['/sbin/lvcreate', '-aly', '-K', '-Wy', '--yes', '--size', $size, '--name', $name];
+    my $cmd = ['/sbin/lvcreate', '-aly', '-K', '-ky', '-Wy', '--yes', '--size', $size, '--name', $name];
     for my $tag (@$tags) {
 	push @$cmd, '--addtag', $tag;
     }
-- 
2.39.2

Re: [pve-devel] [PATCH storage 2/2] fix #4997: lvm: set "activation skip" flag for newly created LVs

[Fiona Ebner]

Am 11.01.24 um 16:03 schrieb Friedrich Weber:
> Activating an LV creates a device-mapper device. In a cluster with a
> shared LVM VG (e.g. on top of iSCSI) 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
> 
> By default, LVM autoactivates all discovered LVs after boot, thus
> creating device-mapper device for all discovered LVs. As a result,
> 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, see [1].
> 
> To avoid this issue in the future, adjust the LVM plugin to create new
> LVs with the "activation skip" flag. LVs with that flag are not
> activated unless `-K` is passed to the activation command. Consequently,
> new LVs will not be autoactivated after boot anymore, and removing LVs
> will not leave behind device-mapper devices on other nodes anymore.
> 
> The LVM plugin is still able to activate LVs, as it already passes
> `-K` to all activation commands.
> 
> 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 set the "activation skip" flag on existing LVs.
> 
> [1] https://bugzilla.proxmox.com/show_bug.cgi?id=4997
> 
> Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> Signed-off-by: Friedrich Weber <f.weber@proxmox.com>

Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>

> ---
> 
> Notes:
>     Should only be applied close to the next major release, see cover
>     letter.
> 
>  src/PVE/Storage/LVMPlugin.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Re: [pve-devel] [PATCH storage 0/2] fix #4997: lvm: avoid autoactivating (new) LVs after boot

[Fiona Ebner]

Am 11.01.24 um 16:03 schrieb Friedrich Weber:
> By default, LVM autoactivates LVs after boot. In a cluster with VM disks on a
> shared LVM VG (e.g. on top of iSCSI), this can indirectly cause guest creation
> or VM live-migration to fail. See bug #4997 [1] and patch #2 for details.
> 
> The goal of this series is to avoid autoactivating LVs after boot. Fabian
> suggested to use the "activation skip" flag for LVs. LVs with that flag can
> only be activated if the `-K` flag is passed during activation (`-K` is not
> passed for autoactivation after boot).
> 
> With patch #1, the LVM plugin passes the `-K` flag to activation commands. If
> the LV does not have the "activation skip" flag set, this should not have any
> effect, so it should be safe to apply this patch in the near future. It does
> not yet fix #4997, though.
> 
> With patch #2, the LVM plugin sets the "activation skip" flag for newly created
> LVs. As this can be considered a breaking change, it may make sense to only
> apply it close to the PVE 9 release. If patch #1 has been available in the last
> PVE 8 minor release, this should ensure a smooth upgrade even if a cluster is
> temporarily mixed between PVE 8.x and 9 (PVE 8.x will be able to activate LVs
> created by PVE 9 with "activation skip"). This will fix #4997 for newly created
> LVs.
> 

Yes, patch #2 can cause issues for clusters with mixed minor versions,
so I'd consider this a breaking change.

> Some points to discuss:
> 
> * Fabian and I discussed whether it may be better to pass `-K` and set the
>   "activation skip" flag only for LVs on a *shared* LVM storage. But this may
>   cause issues for users that incorrectly mark an LVM storage as shared, create a
>   bunch of LVs (with "activation skip" flag), then unset the "shared" flag, and
>   won't be able to activate LVs afterwards (`lvchange -ay` without `-K` on an LV
>   with "activation skip" is a noop). What do you think?
> 

Is there a way to prevent auto-activation on boot for LVs on a shared
(PVE-managed) LVM storage? Also a breaking change, because users might
have other LVs on the same storage, but would avoid the need for the
flag. Not against the current approach, just wondering.

Guardrails against issues caused by misconfiguration always warrant a
cost-benefits analysis. What is the cost for also setting the flag for
LVs on non-shared LVM storages? Or logic needs to be correct either way ;)

> * Even with patch #1 and #2 applied, users can still run into #4997 for LVs
>   that were created before #2, so without the "activation skip" flag. So it might
>   be good to include a note in the 8->9 release notes and/or a warning in the
>   pve8to9 helper and/or ship a script that automatically sets the flag for all
>   existing (PVE-owned) LVs.
> 

Sure, mentioning the changed behavior and suggestion/script for making
it consistent make sense.

Re: [pve-devel] [PATCH storage 0/2] fix #4997: lvm: avoid autoactivating (new) LVs after boot

[Friedrich Weber]

Thanks for the review!

On 26/01/2024 12:14, Fiona Ebner wrote:
>> Some points to discuss:
>>
>> * Fabian and I discussed whether it may be better to pass `-K` and set the
>>   "activation skip" flag only for LVs on a *shared* LVM storage. But this may
>>   cause issues for users that incorrectly mark an LVM storage as shared, create a
>>   bunch of LVs (with "activation skip" flag), then unset the "shared" flag, and
>>   won't be able to activate LVs afterwards (`lvchange -ay` without `-K` on an LV
>>   with "activation skip" is a noop). What do you think?
>>
> 
> Is there a way to prevent auto-activation on boot for LVs on a shared
> (PVE-managed) LVM storage? Also a breaking change, because users might
> have other LVs on the same storage, but would avoid the need for the
> flag. Not against the current approach, just wondering.

One can also disable autoactivation for a whole VG (i.e., all LVs of
that VG):

	vgchange --setautoactivation n VG

At least in my tests, after setting this no LV in that VG is active
after boot, so this might also solve the problem. I suppose setting this
automatically for existing VGs would be too dangerous (as users might
have other LVs in that VGs). But our LVM storage plugin *could* set this
when creating a new shared VG [1]?

Not sure which option is better, though.

> Guardrails against issues caused by misconfiguration always warrant a
> cost-benefits analysis. What is the cost for also setting the flag for
> LVs on non-shared LVM storages? Or logic needs to be correct either way ;)

AFAICT, setting this LV flag on non-shared LVM storages doesn't have
negative side-effects. I don't think we rely on autoactivation anywhere.
We'd need to take care of passing `-K` for all our `lvchange -ay` calls,
but AFAICT, `lvchange` calls are only done in the LVM/LvmThin plugins in
pve-storage.

[1]
https://git.proxmox.com/?p=pve-storage.git;a=blob;f=src/PVE/Storage/LVMPlugin.pm;h=4b951e7a;hb=8289057e#l94

Re: [pve-devel] [PATCH storage 0/2] fix #4997: lvm: avoid autoactivating (new) LVs after boot

[Fiona Ebner]

Am 31.01.24 um 16:07 schrieb Friedrich Weber:
> Thanks for the review!
> 
> On 26/01/2024 12:14, Fiona Ebner wrote:
>>> Some points to discuss:
>>>
>>> * Fabian and I discussed whether it may be better to pass `-K` and set the
>>>   "activation skip" flag only for LVs on a *shared* LVM storage. But this may
>>>   cause issues for users that incorrectly mark an LVM storage as shared, create a
>>>   bunch of LVs (with "activation skip" flag), then unset the "shared" flag, and
>>>   won't be able to activate LVs afterwards (`lvchange -ay` without `-K` on an LV
>>>   with "activation skip" is a noop). What do you think?
>>>
>>
>> Is there a way to prevent auto-activation on boot for LVs on a shared
>> (PVE-managed) LVM storage? Also a breaking change, because users might
>> have other LVs on the same storage, but would avoid the need for the
>> flag. Not against the current approach, just wondering.
> 
> One can also disable autoactivation for a whole VG (i.e., all LVs of
> that VG):
> 
> 	vgchange --setautoactivation n VG
> 
> At least in my tests, after setting this no LV in that VG is active
> after boot, so this might also solve the problem. I suppose setting this
> automatically for existing VGs would be too dangerous (as users might
> have other LVs in that VGs). But our LVM storage plugin *could* set this
> when creating a new shared VG [1]?
> 
> Not sure which option is better, though.
> 

Do you still need the -K flag to activate volumes in such a VG? If yes,
nothing is gained compared to the more fine-grained "setting it on
individual LVs". If no, we could save that. OTOH, users might want to
use existing shared VGs and then they would need to apply this setting
themselves.

>> Guardrails against issues caused by misconfiguration always warrant a
>> cost-benefits analysis. What is the cost for also setting the flag for
>> LVs on non-shared LVM storages? Or logic needs to be correct either way ;)
> 
> AFAICT, setting this LV flag on non-shared LVM storages doesn't have
> negative side-effects. I don't think we rely on autoactivation anywhere.
> We'd need to take care of passing `-K` for all our `lvchange -ay` calls,
> but AFAICT, `lvchange` calls are only done in the LVM/LvmThin plugins in
> pve-storage.
> 

We need to have -K for activations, no matter if we only set the
activationskip flag for shared or all. That's not an additional cost.

Re: [pve-devel] [PATCH storage 0/2] fix #4997: lvm: avoid autoactivating (new) LVs after boot

[Friedrich Weber]

On 01/02/2024 09:26, Fiona Ebner wrote:
> Am 31.01.24 um 16:07 schrieb Friedrich Weber:
>> Thanks for the review!
>>
>> On 26/01/2024 12:14, Fiona Ebner wrote:
>>>> Some points to discuss:
>>>>
>>>> * Fabian and I discussed whether it may be better to pass `-K` and set the
>>>>   "activation skip" flag only for LVs on a *shared* LVM storage. But this may
>>>>   cause issues for users that incorrectly mark an LVM storage as shared, create a
>>>>   bunch of LVs (with "activation skip" flag), then unset the "shared" flag, and
>>>>   won't be able to activate LVs afterwards (`lvchange -ay` without `-K` on an LV
>>>>   with "activation skip" is a noop). What do you think?
>>>>
>>>
>>> Is there a way to prevent auto-activation on boot for LVs on a shared
>>> (PVE-managed) LVM storage? Also a breaking change, because users might
>>> have other LVs on the same storage, but would avoid the need for the
>>> flag. Not against the current approach, just wondering.
>>
>> One can also disable autoactivation for a whole VG (i.e., all LVs of
>> that VG):
>>
>> 	vgchange --setautoactivation n VG
>>
>> At least in my tests, after setting this no LV in that VG is active
>> after boot, so this might also solve the problem. I suppose setting this
>> automatically for existing VGs would be too dangerous (as users might
>> have other LVs in that VGs). But our LVM storage plugin *could* set this
>> when creating a new shared VG [1]?
>>
>> Not sure which option is better, though.
>>
> 
> Do you still need the -K flag to activate volumes in such a VG? If yes,
> nothing is gained compared to the more fine-grained "setting it on
> individual LVs". If no, we could save that. OTOH, users might want to
> use existing shared VGs and then they would need to apply this setting
> themselves.

Just looked into this again: No, if I set `--setautoactivation n` for a
VG, I don't need to pass -K to activate LVs within that VG. I think the
`--setautoactivation n` flag for VGs/LVs only affects autoactivation
(e.g. `vgchange -aay`, as done by udev, or `lvchange -aay`), not manual
activation (e.g. `vgchange -ay`/`lvchange -ay`).

> 
>>> Guardrails against issues caused by misconfiguration always warrant a
>>> cost-benefits analysis. What is the cost for also setting the flag for
>>> LVs on non-shared LVM storages? Or logic needs to be correct either way ;)
>>
>> AFAICT, setting this LV flag on non-shared LVM storages doesn't have
>> negative side-effects. I don't think we rely on autoactivation anywhere.
>> We'd need to take care of passing `-K` for all our `lvchange -ay` calls,
>> but AFAICT, `lvchange` calls are only done in the LVM/LvmThin plugins in
>> pve-storage.
>>
> 
> We need to have -K for activations, no matter if we only set the
> activationskip flag for shared or all. That's not an additional cost.



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

Re: [pve-devel] [PATCH storage 0/2] fix #4997: lvm: avoid autoactivating (new) LVs after boot

[Friedrich Weber]

On 11/01/2024 16:03, Friedrich Weber wrote:
> By default, LVM autoactivates LVs after boot. In a cluster with VM disks on a
> shared LVM VG (e.g. on top of iSCSI), this can indirectly cause guest creation
> or VM live-migration to fail. See bug #4997 [1] and patch #2 for details.
> 
> The goal of this series is to avoid autoactivating LVs after boot. Fabian
> suggested to use the "activation skip" flag for LVs. LVs with that flag can
> only be activated if the `-K` flag is passed during activation (`-K` is not
> passed for autoactivation after boot).

I revisited this issue now, and with some distance, I wondered why
to go with the quite heavy-handed approach of

- (1) activating LVs with -K/--ignoreactivationskip
- (2) starting with PVE 9, creating LVs with -k/`--setactivationskip y`

If it would be possible to instead just create new LVs with
`--setautoactivation n`, we wouldn't need to touch the activation code
at all, and wouldn't need to worry about the mixed-version cluster
scenario.

For VGs on iSCSI volumes, setting `--setautoactivation n` on LVs seems
to work fine -- this way, after iSCSI login, these volumes are not
automatically activated. The reason is that autoactivation is done by
udev via /lib/udev/rules.d/69-lvm.rules:

	IMPORT{program}="/sbin/lvm pvscan --cache --listvg --checkcomplete --vgonline --autoactivation event --udevoutput --journal=output $env{DEVNAME}"
	TEST!="/run/systemd/system", GOTO="lvm_direct_vgchange"

	ENV{LVM_VG_NAME_COMPLETE}=="?*", RUN+="/usr/bin/systemd-run --no-block --property DefaultDependencies=no --unit lvm-activate-$env{LVM_VG_NAME_COMPLETE} /sbin/lvm vgchange -aay --autoactivation event $env{LVM_VG_NAME_COMPLETE}"
	GOTO="lvm_end"

	LABEL="lvm_direct_vgchange"
	ENV{LVM_VG_NAME_COMPLETE}=="?*", RUN+="/sbin/lvm vgchange -aay --autoactivation event $env{LVM_VG_NAME_COMPLETE}"
	GOTO="lvm_end"

In both branches vgchange is called with `-aay` where `ay` specifies
autoactivation, which according to `man vgchange` doesn't activate LVs
with `--setautoactivation n`.

However, for VGs on FC/SAS+multipath disks (these are visible at boot
time), even an LV with `--setautoactivation n` is active after boot.
I was curious why, and I *think* (see [2] for info how to debug this)
it's because they are already activated in initramfs (because they are
visible at boot time):

- 69-lvm.conf does in fact call vgchange with `-aay`, and does not
activate LVs with `--setautoactivation n`, as expected

- but zfs-initramfs also installs a script
/usr/share/initramfs-tools/scripts/local-top/zfs [1] to initramfs which
calls:

        /sbin/lvm vgscan
        /sbin/lvm vgchange -a y --sysinit

Note that it calls `vgchange -ay` (instead of `vgchange -aay`) so LVM
doesn't consider this autoactivation and activates all LVs regardless of
their `--setautoactivation` flag. If I edit the script to use `-aay`
instead, the LV with `--setautoactivation n` flag is inactive after boot
as expected.

So I'm wondering:

(a) could the ZFS initramfs script use `-aay` instead of `-ay`, so the
`--setautoactivation` flag has an effect again for LVs that are visible
at boot?

(b) assuming (a) can be fixed, is there any additional reason to prefer
to --ignoreactivationskip/--setactivationskip approach over the
`--setautoactivation n` approach?

[1]
https://github.com/openzfs/zfs/blob/c2458ba921a8d59cd6b705f81b37f50531f8670b/contrib/initramfs/scripts/local-top/zfs

[2] FWIW, one way to debug this:

- attaching a serial console to the VM
- removing `quiet` and adding `console=ttyS0` to the kernel command line
- setting `log_level=debug` in
/usr/share/initramfs-tools/scripts/init-top/udev
- adding some debug printouts (echo XYZ > /dev/ttyS0) to
/usr/share/initramfs-tools/scripts/local-top/zfs
- don't forget `update-initramfs -u`
- attach to serial console on host: `qm console VMID`



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

Re: [pve-devel] [PATCH storage 0/2] fix #4997: lvm: avoid autoactivating (new) LVs after boot

[Fabian Grünbichler]

On February 7, 2025 1:44 pm, Friedrich Weber wrote:
> On 11/01/2024 16:03, Friedrich Weber wrote:
>> By default, LVM autoactivates LVs after boot. In a cluster with VM disks on a
>> shared LVM VG (e.g. on top of iSCSI), this can indirectly cause guest creation
>> or VM live-migration to fail. See bug #4997 [1] and patch #2 for details.
>> 
>> The goal of this series is to avoid autoactivating LVs after boot. Fabian
>> suggested to use the "activation skip" flag for LVs. LVs with that flag can
>> only be activated if the `-K` flag is passed during activation (`-K` is not
>> passed for autoactivation after boot).
> 
> I revisited this issue now, and with some distance, I wondered why
> to go with the quite heavy-handed approach of
> 
> - (1) activating LVs with -K/--ignoreactivationskip
> - (2) starting with PVE 9, creating LVs with -k/`--setactivationskip y`
> 
> If it would be possible to instead just create new LVs with
> `--setautoactivation n`, we wouldn't need to touch the activation code
> at all, and wouldn't need to worry about the mixed-version cluster
> scenario.
> 
> For VGs on iSCSI volumes, setting `--setautoactivation n` on LVs seems
> to work fine -- this way, after iSCSI login, these volumes are not
> automatically activated. The reason is that autoactivation is done by
> udev via /lib/udev/rules.d/69-lvm.rules:
> 
> 	IMPORT{program}="/sbin/lvm pvscan --cache --listvg --checkcomplete --vgonline --autoactivation event --udevoutput --journal=output $env{DEVNAME}"
> 	TEST!="/run/systemd/system", GOTO="lvm_direct_vgchange"
> 
> 	ENV{LVM_VG_NAME_COMPLETE}=="?*", RUN+="/usr/bin/systemd-run --no-block --property DefaultDependencies=no --unit lvm-activate-$env{LVM_VG_NAME_COMPLETE} /sbin/lvm vgchange -aay --autoactivation event $env{LVM_VG_NAME_COMPLETE}"
> 	GOTO="lvm_end"
> 
> 	LABEL="lvm_direct_vgchange"
> 	ENV{LVM_VG_NAME_COMPLETE}=="?*", RUN+="/sbin/lvm vgchange -aay --autoactivation event $env{LVM_VG_NAME_COMPLETE}"
> 	GOTO="lvm_end"
> 
> In both branches vgchange is called with `-aay` where `ay` specifies
> autoactivation, which according to `man vgchange` doesn't activate LVs
> with `--setautoactivation n`.
> 
> However, for VGs on FC/SAS+multipath disks (these are visible at boot
> time), even an LV with `--setautoactivation n` is active after boot.
> I was curious why, and I *think* (see [2] for info how to debug this)
> it's because they are already activated in initramfs (because they are
> visible at boot time):
> 
> - 69-lvm.conf does in fact call vgchange with `-aay`, and does not
> activate LVs with `--setautoactivation n`, as expected
> 
> - but zfs-initramfs also installs a script
> /usr/share/initramfs-tools/scripts/local-top/zfs [1] to initramfs which
> calls:
> 
>         /sbin/lvm vgscan
>         /sbin/lvm vgchange -a y --sysinit
> 
> Note that it calls `vgchange -ay` (instead of `vgchange -aay`) so LVM
> doesn't consider this autoactivation and activates all LVs regardless of
> their `--setautoactivation` flag. If I edit the script to use `-aay`
> instead, the LV with `--setautoactivation n` flag is inactive after boot
> as expected.

thanks for the detailed analysis/follow-up!

> 
> So I'm wondering:
> 
> (a) could the ZFS initramfs script use `-aay` instead of `-ay`, so the
> `--setautoactivation` flag has an effect again for LVs that are visible
> at boot?

probably ;)

> 
> (b) assuming (a) can be fixed, is there any additional reason to prefer
> to --ignoreactivationskip/--setactivationskip approach over the
> `--setautoactivation n` approach?

the only other reason I could think of is that there's a metric ton of
tutorials/howtos for various things that contain "vgchange -ay" which
would still entail potential breakage.. but then again, one could argue
that people blindly following such instructions are to blame, and the
reduced complexity on our end would be worth it..

> 
> [1]
> https://github.com/openzfs/zfs/blob/c2458ba921a8d59cd6b705f81b37f50531f8670b/contrib/initramfs/scripts/local-top/zfs
> 
> [2] FWIW, one way to debug this:
> 
> - attaching a serial console to the VM
> - removing `quiet` and adding `console=ttyS0` to the kernel command line
> - setting `log_level=debug` in
> /usr/share/initramfs-tools/scripts/init-top/udev
> - adding some debug printouts (echo XYZ > /dev/ttyS0) to
> /usr/share/initramfs-tools/scripts/local-top/zfs
> - don't forget `update-initramfs -u`
> - attach to serial console on host: `qm console VMID`
> 
> 


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

Re: [pve-devel] [PATCH storage 0/2] fix #4997: lvm: avoid autoactivating (new) LVs after boot

[Friedrich Weber]

sent a v2:
https://lore.proxmox.com/pve-devel/20250307095245.65698-1-f.weber@proxmox.com/T/

On 10/02/2025 11:47, Fabian Grünbichler wrote:
> [...]
> 
>>
>> So I'm wondering:
>>
>> (a) could the ZFS initramfs script use `-aay` instead of `-ay`, so the
>> `--setautoactivation` flag has an effect again for LVs that are visible
>> at boot?
> 
> probably ;)

v2 contains a zfsonlinux patch for this, I'll also send it upstream.

>>
>> (b) assuming (a) can be fixed, is there any additional reason to prefer
>> to --ignoreactivationskip/--setactivationskip approach over the
>> `--setautoactivation n` approach?
> 
> the only other reason I could think of is that there's a metric ton of
> tutorials/howtos for various things that contain "vgchange -ay" which
> would still entail potential breakage.. but then again, one could argue
> that people blindly following such instructions are to blame, and the
> reduced complexity on our end would be worth it..

Good point, though I'd hope users would run `vgchange -ay` only
occasionally, triggering the issue for the current boot only but not for
every subsequent boot. The v2 would probably still need to wait until
PVE 9 (see its cover letter for details), so not much gained here in
comparison to v1. I still find its approach less complex and would
prefer it, but this is of course debatable :)

>>
>> [1]
>> https://github.com/openzfs/zfs/blob/c2458ba921a8d59cd6b705f81b37f50531f8670b/contrib/initramfs/scripts/local-top/zfs
>>
>> [2] FWIW, one way to debug this:
>>
>> - attaching a serial console to the VM
>> - removing `quiet` and adding `console=ttyS0` to the kernel command line
>> - setting `log_level=debug` in
>> /usr/share/initramfs-tools/scripts/init-top/udev
>> - adding some debug printouts (echo XYZ > /dev/ttyS0) to
>> /usr/share/initramfs-tools/scripts/local-top/zfs
>> - don't forget `update-initramfs -u`
>> - attach to serial console on host: `qm console VMID`
>>
>>
> 
> 
> _______________________________________________
> 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