[PATCH v3 storage 05/76] lvm: find a shared volume group created by another node

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

LVM hints restrict scans to devices that previously held PVs. A shared
volume group created by another node on an already attached device can
remain invisible until the hints are invalidated, leaving the storage
inactive and blocking guest startup and migration to that node. The
fallback vgscan finds the group but updates neither the hints nor the
cached volume-group list.

Retry the listing without hints. If it finds additional groups, rebuild
the hints with 'pvscan --cache', update the cached groups, and
invalidate the logical-volume list used by thin pool activation. Rebuild
only when needed: pvscan also resets LVM autoactivation records and must
not run on every status pass merely because a group remains absent.

This was observed on a five-node cluster with shared groups created on
iSCSI multipath LUNs and NVMe over Fabrics namespaces already attached
to the other nodes.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 src/PVE/Storage/LVMPlugin.pm   |  43 ++++++++-----
 src/test/Makefile              |   5 +-
 src/test/run_test_lvm_hints.pl | 109 +++++++++++++++++++++++++++++++++
 3 files changed, 140 insertions(+), 17 deletions(-)
 create mode 100755 src/test/run_test_lvm_hints.pl

diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index a6a5eb70..3ed5bc78 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -161,7 +161,7 @@ sub lvm_destroy_volume_group {
 }
 
 sub lvm_vgs {
-    my ($includepvs) = @_;
+    my ($includepvs, $nohints) = @_;
 
     my $cmd = [
         '/sbin/vgs',
@@ -172,8 +172,9 @@ sub lvm_vgs {
         'b',
         '--unbuffered',
         '--nosuffix',
-        '--options',
     ];
+    push @$cmd, '--nohints' if $nohints;
+    push @$cmd, '--options';
 
     my $cols = [qw(vg_name vg_size vg_free lv_count)];
 
@@ -1420,25 +1421,35 @@ sub volume_snapshot_info {
     return $info;
 }
 
+# LVM hints can miss PVs written by another node onto previously empty devices. Probe without
+# hints once per activation cache; rebuild only when new VGs appear, since rebuilding also resets
+# LVM autoactivation records.
+my sub find_unhinted_vgs {
+    my ($cache) = @_;
+
+    $cache->{vgscaned} = 1;
+
+    my $vgs = lvm_vgs(0, 1);
+    my @unhinted = grep { !$cache->{vgs}->{$_} } sort keys %$vgs;
+    return if !@unhinted;
+
+    eval {
+        run_command(['/sbin/pvscan', '--cache'], outfunc => sub { });
+    };
+    warn "rebuilding the LVM device hints failed - $@" if $@;
+
+    # add, never replace: an incomplete listing must not hide volume groups found before
+    $cache->{vgs}->{$_} = $vgs->{$_} for @unhinted;
+    # the cached logical volume list lacks the volumes of these volume groups
+    delete $cache->{lvs};
+}
+
 sub activate_storage {
     my ($class, $storeid, $scfg, $cache) = @_;
 
     $cache->{vgs} = lvm_vgs() if !$cache->{vgs};
 
-    # In LVM2, vgscans take place automatically;
-    # this is just to be sure
-    if (
-        $cache->{vgs}
-        && !$cache->{vgscaned}
-        && !$cache->{vgs}->{ $scfg->{vgname} }
-    ) {
-        $cache->{vgscaned} = 1;
-        my $cmd = ['/sbin/vgscan', '--ignorelockingfailure', '--mknodes'];
-        eval {
-            run_command($cmd, outfunc => sub { });
-        };
-        warn $@ if $@;
-    }
+    find_unhinted_vgs($cache) if !$cache->{vgscaned} && !$cache->{vgs}->{ $scfg->{vgname} };
 
     # we do not acticate any volumes here ('vgchange -aly')
     # instead, volumes are activate individually later
diff --git a/src/test/Makefile b/src/test/Makefile
index 1fe50885..02d833e8 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -1,6 +1,6 @@
 all: test
 
-test: test_iscsi test_zfspoolplugin test_lvmplugin test_lvm_lock_retry test_lvm_base_volume test_disklist test_bwlimit test_plugin test_ovf test_volume_access
+test: test_iscsi test_zfspoolplugin test_lvmplugin test_lvm_lock_retry test_lvm_base_volume test_lvm_hints test_disklist test_bwlimit test_plugin test_ovf test_volume_access
 
 test_zfspoolplugin: run_test_zfspoolplugin.pl
 	./run_test_zfspoolplugin.pl
@@ -15,6 +15,9 @@ test_lvm_lock_retry: run_test_lvm_lock_retry.pl
 test_lvm_base_volume: run_test_lvm_base_volume.pl
 	./run_test_lvm_base_volume.pl
 
+test_lvm_hints: run_test_lvm_hints.pl
+	./run_test_lvm_hints.pl
+
 test_disklist: run_disk_tests.pl
 	./run_disk_tests.pl
 
diff --git a/src/test/run_test_lvm_hints.pl b/src/test/run_test_lvm_hints.pl
new file mode 100755
index 00000000..fc2ffff7
--- /dev/null
+++ b/src/test/run_test_lvm_hints.pl
@@ -0,0 +1,109 @@
+#!/usr/bin/perl
+
+# Unit test for the activation of an LVM storage whose volume group is missing from the volume
+# groups LVM lists through its device hints, as after another node created it on a device this node
+# already had: a listing without the hints finds it, and only then the hints are rebuilt. vgs and
+# pvscan are mocked, so the test needs neither root nor a real LVM setup.
+
+use strict;
+use warnings;
+
+use lib '..';
+use Test::More;
+use Test::MockModule;
+
+use PVE::Storage::LVMPlugin;
+use PVE::Storage::LvmThinPlugin;
+
+my $CLASS = 'PVE::Storage::LVMPlugin';
+
+# the volume groups vgs lists with and without the hints, and the commands run
+my ($hinted, $all, $vgs_fails, $pvscan_fails, @calls);
+
+my $lvm = Test::MockModule->new('PVE::Storage::LVMPlugin');
+$lvm->mock(
+    run_command => sub {
+        my ($cmd, %param) = @_;
+        if ($cmd->[0] eq '/sbin/vgs') {
+            my $nohints = grep { $_ eq '--nohints' } @$cmd;
+            push @calls, $nohints ? 'vgs --nohints' : 'vgs';
+            die "vgs failed\n" if $nohints && $vgs_fails;
+            $param{outfunc}->("$_:1024:512:0") for ($nohints ? $all : $hinted)->@*;
+        } elsif ($cmd->[0] eq '/sbin/pvscan') {
+            push @calls, 'pvscan --cache';
+            die "pvscan failed\n" if $pvscan_fails;
+        } else {
+            push @calls, join(' ', @$cmd);
+        }
+    },
+);
+
+my sub activate {
+    my ($vgname, $cache) = @_;
+
+    @calls = ();
+    my @warnings;
+    local $SIG{__WARN__} = sub { push @warnings, $_[0] };
+    $CLASS->activate_storage('lvm', { vgname => $vgname }, $cache);
+    return (join(', ', @calls), \@warnings);
+}
+
+($hinted, $all) = (['vg1'], ['vg1']);
+my ($calls) = activate('vg1', {});
+is($calls, 'vgs', 'listed volume group: no listing without the hints');
+
+($hinted, $all) = (['vg1'], ['vg1', 'vg2']);
+my $cache = { lvs => { vg1 => {} } };
+($calls) = activate('vg2', $cache);
+is($calls, 'vgs, vgs --nohints, pvscan --cache', 'unhinted volume group: hints rebuilt');
+ok($cache->{vgs}->{vg2} && $cache->{vgs}->{vg1}, 'and added to the volume groups of the cache');
+ok(!exists($cache->{lvs}), 'and the logical volume list of the cache is dropped');
+is_deeply(
+    [$CLASS->status('lvm', { vgname => 'vg2' }, $cache)],
+    [1024, 512, 512, 1],
+    'the status of the same cache sees the volume group',
+);
+
+($calls) = activate('vg3', $cache);
+is($calls, '', 'once per cache: no second listing for the next missing volume group');
+
+($hinted, $all) = (['vg1'], ['vg1']);
+($calls) = activate('vg2', {});
+is($calls, 'vgs, vgs --nohints', 'volume group missing everywhere: hints left alone');
+
+($hinted, $all, $vgs_fails) = (['vg1'], [], 1);
+$cache = {};
+my $warnings;
+($calls, $warnings) = activate('vg2', $cache);
+is($calls, 'vgs, vgs --nohints', 'failed listing without the hints: hints left alone');
+ok($cache->{vgs}->{vg1}, 'and the volume groups found before stay');
+like($warnings->[0] // '', qr/vgs failed/, 'and it warns');
+$vgs_fails = 0;
+
+($hinted, $all, $pvscan_fails) = (['vg1'], ['vg1', 'vg2'], 1);
+$cache = {};
+($calls, $warnings) = activate('vg2', $cache);
+ok($cache->{vgs}->{vg2}, 'failed rebuild of the hints: the volume group still counts');
+like(
+    $warnings->[0] // '',
+    qr/^rebuilding the LVM device hints failed - pvscan failed/,
+    'and it warns about the rebuild',
+);
+$pvscan_fails = 0;
+
+# a thin pool storage activates through the LVM activation and reads the logical volume list
+my $listed;
+$lvm->mock(
+    lvm_list_volumes => sub {
+        $listed++;
+        return { vg2 => { data => { lv_type => 't', lv_state => 'a' } } };
+    },
+);
+($hinted, $all) = (['vg1'], ['vg1', 'vg2']);
+$cache = { lvs => { vg1 => {} } };
+PVE::Storage::LvmThinPlugin->activate_storage(
+    'thin', { vgname => 'vg2', thinpool => 'data' }, $cache,
+);
+ok($listed, 'thin pool: the logical volumes are listed again after the volume group was found');
+
+done_testing();
-- 
2.47.3