Re: [pve-devel] [PATCH manager/storage v4 0/5] fix #4997: lvm, lvm-thin: avoid autoactivating LVs

From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Friedrich Weber <f.weber@proxmox.com>
Subject: Re: [pve-devel] [PATCH manager/storage v4 0/5] fix #4997: lvm, lvm-thin: avoid autoactivating LVs
Date: Tue, 8 Jul 2025 10:38:00 +0200	[thread overview]
Message-ID: <cf065c51-4a9a-4f28-837a-db5399349cd3@proxmox.com> (raw)
In-Reply-To: <20250707080352.58713-1-f.weber@proxmox.com>

Thanks for your polished submission, very easy to digest!

Am 07.07.25 um 10:03 schrieb Friedrich Weber:
> # pve8to9 script
> 
> As discussed in v2, this series implements
> 
> (a) a pve8to9 check to detect thick and thin LVs with autoactivation enabled
> (b) a script to disable autoactivation on LVs when needed, intended to be run
>     manually by the user during 8->9 upgrade
> 
> The question is where to put the script (b). Patch #4 moves the existing checks
> from `pve8to9` to `pve8to9 checklist`, to be able to implement (b) as a new
> subcommand `pve8to9 updatelvm`. I realize this is a huge user-facing change,
> and we don't have to go with this approach. It is also incomplete, as patch #5
> doesn't update the manpage yet. However, I like about this approach that
> pve8to9 bundles "tasks that are related to 8->9 upgrades". If we do decide to
> go with this, I can send another patch to update the manpage as well as add
> documentation.

I'd prefer shipping the update in it's own script and outside of path.
We should ship that either below /usr/libexec, or–maybe even nicer–in a
package specific /usr/share path, like e.g. inside:
/usr/share/pve-manager/migrations/...

The checker script would then output the respective path to use.

Keeping such migration scripts, that actively change the host, independent
from the XtoY scripts avoids the need for multiple copies for multiple XtoY
scripts, as often the next future version might benefit from still having
these. Having a strict boundary between idempotent checks and code that
actually changes the hosts seems a bit safer to me too, especially if the
latter is not accessible via $PATH and also because we execute commands even
if they are not specified completely but if the user supplied string uniquely
matches the start of an existing command, like "u" or "update" in this case
here would be enough to trigger the command.

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