[pve-devel] [PATCH-SERIES manager 0/2] lvm config: check that --clear-needs-check-flag is set if there is a thin_check_options override

[pve-devel] [PATCH-SERIES manager 0/2] lvm config: check that --clear-needs-check-flag is set if there is a thin_check_options override

[Fiona Ebner]

Quoting the commit message from [0] verbatim:

> thin_check v1.0.x reveals data block ref count issue that is not being
> detected by previous versions, which blocks the pool from activation if
> there are any leaked blocks. To reduce potential user complaints on
> inactive pools after upgrading and also maintain backward compatibility
> between LVM and older thin_check, we decided to adopt the 'auto-repair'
> functionality in the --clear-needs-check-flag option, rather than
> passing --auto-repair from lvm.conf.

Unfortunately, there was already a user report without the override
[1], so this might not be the only issue. It's still worth warning
users about this though.

NOTE: For stable-8 the version check in d/postinst needs to be adapted
or maybe we want to run the check always there?

[0]: https://github.com/device-mapper-utils/thin-provisioning-tools/commit/eb28ab94
[1]: https://forum.proxmox.com/threads/169356/post-789894


Fiona Ebner (2):
  8 to 9: lvm config: check that --clear-needs-check-flag is set if
    there is a thin_check_options override
  d/postinst: lvm config: check that --clear-needs-check-flag is set if
    there is a thin_check_options override

 PVE/CLI/pve8to9.pm | 27 +++++++++++++++++++++++++++
 debian/postinst    |  8 ++++++++
 2 files changed, 35 insertions(+)

-- 
2.47.2



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

[pve-devel] [PATCH manager 1/2] 8 to 9: lvm config: check that --clear-needs-check-flag is set if there is a thin_check_options override

[Fiona Ebner]

Quoting the commit message from [0] verbatim:

thin_check v1.0.x reveals data block ref count issue that is not being
detected by previous versions, which blocks the pool from activation if
there are any leaked blocks. To reduce potential user complaints on
inactive pools after upgrading and also maintain backward compatibility
between LVM and older thin_check, we decided to adopt the 'auto-repair'
functionality in the --clear-needs-check-flag option, rather than
passing --auto-repair from lvm.conf.

[0]: https://github.com/device-mapper-utils/thin-provisioning-tools/commit/eb28ab94

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/CLI/pve8to9.pm | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/PVE/CLI/pve8to9.pm b/PVE/CLI/pve8to9.pm
index dc3bf9ea..dcae429e 100644
--- a/PVE/CLI/pve8to9.pm
+++ b/PVE/CLI/pve8to9.pm
@@ -1796,6 +1796,32 @@ sub check_lvm_autoactivation {
     return undef;
 }
 
+sub check_lvm_thin_check_options {
+    log_info("Checking lvm config for thin_check_options...");
+
+    my $section;
+    my $detected;
+    my $detect_thin_check_override = sub {
+        my $line = shift;
+        if ($line =~ m/^(\S+) \{/) {
+            $section = $1;
+            return;
+        }
+        if ($line =~ m/thin_check_options/ && $line !~ m/--clear-needs-check-flag/) {
+            $detected = 1;
+            log_fail(
+                "detected override for 'thin_check_options' in '$section' section without"
+                    . " '--clear-needs-check-flag' option - add the option to your override (most"
+                    . " likely in /etc/lvm/lvm.conf)");
+        }
+    };
+    eval {
+        run_command(['lvmconfig'], outfunc => $detect_thin_check_override);
+        log_pass("Check for correct thin_check_options passed") if !$detected;
+    };
+    log_fail("unable to run 'lvmconfig' command - $@") if $@;
+}
+
 sub check_glusterfs_storage_usage {
     my $cfg = PVE::Storage::config();
     my $storage_info = PVE::Storage::storage_info($cfg);
@@ -2229,6 +2255,7 @@ sub check_misc {
     check_legacy_notification_sections();
     check_legacy_backup_job_options();
     check_lvm_autoactivation();
+    check_lvm_thin_check_options();
     check_rrd_migration();
     check_legacy_ipam_files();
     check_legacy_sysctl_conf();
-- 
2.47.2



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

[pve-devel] [PATCH manager 2/2] d/postinst: lvm config: check that --clear-needs-check-flag is set if there is a thin_check_options override

[Fiona Ebner]

Quoting the commit message from [0] verbatim:

thin_check v1.0.x reveals data block ref count issue that is not being
detected by previous versions, which blocks the pool from activation if
there are any leaked blocks. To reduce potential user complaints on
inactive pools after upgrading and also maintain backward compatibility
between LVM and older thin_check, we decided to adopt the 'auto-repair'
functionality in the --clear-needs-check-flag option, rather than
passing --auto-repair from lvm.conf.

[0]: https://github.com/device-mapper-utils/thin-provisioning-tools/commit/eb28ab94

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 debian/postinst | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/debian/postinst b/debian/postinst
index b1761c24..b6e07fd9 100755
--- a/debian/postinst
+++ b/debian/postinst
@@ -238,6 +238,14 @@ case "$1" in
         /usr/libexec/proxmox/proxmox-rrd-migration-tool --migrate || \
             echo "migration failed, see output above for errors and try to migrate existing data manually by running '/usr/libexec/proxmox/proxmox-rrd-migration-tool --migrate'"
     fi
+
+    if test -n "$2" && dpkg --compare-versions "$2" 'lt' '9.0.4'; then
+        if test -e /etc/lvm/lvm.conf && grep "^\s*thin_check_options" /etc/lvm/lvm.conf | grep -qv -- "--clear-needs-check-flag"; then
+            printf '\nNOTE: Detected override for 'thin_check_options' without '--clear-needs-check-flag' option in /etc/lvm/lvm.conf\n'
+            printf 'Add the option to the override or thin pools with minor issues might not automatically activate anymore!\n\n'
+        fi
+    fi
+
     ;;
 
   abort-upgrade|abort-remove|abort-deconfigure)
-- 
2.47.2



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

Re: [pve-devel] [PATCH-SERIES manager 0/2] lvm config: check that --clear-needs-check-flag is set if there is a thin_check_options override

[Fabian Grünbichler]

On August 8, 2025 4:03 pm, Fiona Ebner wrote:
> Quoting the commit message from [0] verbatim:
> 
>> thin_check v1.0.x reveals data block ref count issue that is not being
>> detected by previous versions, which blocks the pool from activation if
>> there are any leaked blocks. To reduce potential user complaints on
>> inactive pools after upgrading and also maintain backward compatibility
>> between LVM and older thin_check, we decided to adopt the 'auto-repair'
>> functionality in the --clear-needs-check-flag option, rather than
>> passing --auto-repair from lvm.conf.
> 
> Unfortunately, there was already a user report without the override
> [1], so this might not be the only issue. It's still worth warning
> users about this though.
> 
> NOTE: For stable-8 the version check in d/postinst needs to be adapted
> or maybe we want to run the check always there?

I don't think we need to run it there at all, do we? the problematic
behaviour only comes with trixie's version of thin_check AFAIU?

> [0]: https://github.com/device-mapper-utils/thin-provisioning-tools/commit/eb28ab94
> [1]: https://forum.proxmox.com/threads/169356/post-789894
> 
> 
> Fiona Ebner (2):
>   8 to 9: lvm config: check that --clear-needs-check-flag is set if
>     there is a thin_check_options override
>   d/postinst: lvm config: check that --clear-needs-check-flag is set if
>     there is a thin_check_options override
> 
>  PVE/CLI/pve8to9.pm | 27 +++++++++++++++++++++++++++
>  debian/postinst    |  8 ++++++++
>  2 files changed, 35 insertions(+)
> 
> -- 
> 2.47.2
> 
> 
> 
> _______________________________________________
> 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

Re: [pve-devel] [PATCH-SERIES manager 0/2] lvm config: check that --clear-needs-check-flag is set if there is a thin_check_options override

[Fiona Ebner]

Am 08.08.25 um 4:19 PM schrieb Fabian Grünbichler:
> On August 8, 2025 4:03 pm, Fiona Ebner wrote:
>> Quoting the commit message from [0] verbatim:
>>
>>> thin_check v1.0.x reveals data block ref count issue that is not being
>>> detected by previous versions, which blocks the pool from activation if
>>> there are any leaked blocks. To reduce potential user complaints on
>>> inactive pools after upgrading and also maintain backward compatibility
>>> between LVM and older thin_check, we decided to adopt the 'auto-repair'
>>> functionality in the --clear-needs-check-flag option, rather than
>>> passing --auto-repair from lvm.conf.
>>
>> Unfortunately, there was already a user report without the override
>> [1], so this might not be the only issue. It's still worth warning
>> users about this though.
>>
>> NOTE: For stable-8 the version check in d/postinst needs to be adapted
>> or maybe we want to run the check always there?
> 
> I don't think we need to run it there at all, do we? the problematic
> behaviour only comes with trixie's version of thin_check AFAIU?

Yes.


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

[pve-devel] applied: [PATCH-SERIES manager 0/2] lvm config: check that --clear-needs-check-flag is set if there is a thin_check_options override

[Fabian Grünbichler]

On Fri, 08 Aug 2025 16:03:22 +0200, Fiona Ebner wrote:
> Quoting the commit message from [0] verbatim:
> 
> > thin_check v1.0.x reveals data block ref count issue that is not being
> > detected by previous versions, which blocks the pool from activation if
> > there are any leaked blocks. To reduce potential user complaints on
> > inactive pools after upgrading and also maintain backward compatibility
> > between LVM and older thin_check, we decided to adopt the 'auto-repair'
> > functionality in the --clear-needs-check-flag option, rather than
> > passing --auto-repair from lvm.conf.
> 
> [...]

Applied, thanks!

[1/2] 8 to 9: lvm config: check that --clear-needs-check-flag is set if there is a thin_check_options override
      commit: d038d7d07e0edea6259a17dc7a952e761746c02d
[2/2] d/postinst: lvm config: check that --clear-needs-check-flag is set if there is a thin_check_options override
      commit: adcc7b208321062f8dc56402f1bfe35844d24392

Best regards,
-- 
Fabian Grünbichler <f.gruenbichler@proxmox.com>


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