From 7b41368fc3c2ab1ce5b94a8ae62a38b295e6361e Mon Sep 17 00:00:00 2001 From: Fiona Ebner Date: Tue, 22 Apr 2025 15:33:14 +0200 Subject: [PATCH] lvm thin plugin: do not combine activation change and property change As reported in the community forum [0], there currently is a warning from LVM when converting to a base image on an LVM-thin storage: > WARNING: Combining activation change with other commands is not advised. From a comment in the LVM source code: > Unfortunately, lvchange has previously allowed changing an LV > property and changing LV activation in a single command. This was > not a good idea because the behavior/results are hard to predict and > not possible to sensibly describe. It's also unnecessary. So, this > is here for the sake of compatibility. > > This is extremely ugly; activation should always be done separately. > This is not the full-featured lvchange capability, just the basic > (the advanced activate options are not provided.) > > FIXME: wrap this in a config setting that we can disable by default > to phase this out? While it's not clear there's an actual issue in the specific use case here, just follow what LVM recommends for future-proofing. [0]: https://forum.proxmox.com/threads/165279/ Signed-off-by: Fiona Ebner Link: https://lore.proxmox.com/20250422133314.60806-1-f.ebner@proxmox.com --- src/PVE/Storage/LvmThinPlugin.pm | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/PVE/Storage/LvmThinPlugin.pm b/src/PVE/Storage/LvmThinPlugin.pm index ccefd0e..2797d9e 100644 --- a/src/PVE/Storage/LvmThinPlugin.pm +++ b/src/PVE/Storage/LvmThinPlugin.pm @@ -340,8 +340,13 @@ sub create_base { my $cmd = ['/sbin/lvrename', $vg, $volname, $newname]; run_command($cmd, errmsg => "lvrename '$vg/$volname' => '$vg/$newname' error"); - # set inactive, read-only and activationskip flags - $cmd = ['/sbin/lvchange', '-an', '-pr', '-ky', "$vg/$newname"]; + # set read-only and activationskip flags + $cmd = ['/sbin/lvchange', '-pr', '-ky', "$vg/$newname"]; + eval { run_command($cmd); }; + warn $@ if $@; + + # LVM warns when changing properties and activation at the same time, so inactivate separately + $cmd = ['/sbin/lvchange', '-an', "$vg/$newname"]; eval { run_command($cmd); }; warn $@ if $@;