[PATCH v3 storage 30/76] lvm: activate a missing base volume before using the volume group

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

Activating LVM storage activates its base storage, but not the base
volume. After login, that device may still be appearing, for example
while multipathd assembles its map. Guest startup then fails to find the
volume group even though the device arrives shortly afterward.

If the group is missing, activate the base volume to wait for its
device, then retry the lookup, bypassing LVM hints if necessary. Skip
this wait during status collection so an absent device cannot stall
every status pass. Activation of an already visible group is unchanged.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 src/PVE/Storage/LVMPlugin.pm             |  30 ++++++
 src/test/Makefile                        |   5 +-
 src/test/run_test_lvm_base_activation.pl | 130 +++++++++++++++++++++++
 3 files changed, 164 insertions(+), 1 deletion(-)
 create mode 100755 src/test/run_test_lvm_base_activation.pl

diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 11a64c97..ed7cdb8c 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -1604,6 +1604,32 @@ my sub find_unhinted_vgs {
     delete $cache->{lvs};
 }
 
+# The base device may still be appearing after transport activation; wait before retrying LVM.
+my sub activate_missing_base {
+    my ($storeid, $scfg, $cache) = @_;
+
+    # the volume group list of the cache can predate the device
+    $cache->{vgs} = lvm_vgs();
+    return if $cache->{vgs}->{ $scfg->{vgname} };
+
+    eval {
+        my $cfg = PVE::Storage::config();
+        my ($baseid, $volname) = PVE::Storage::Plugin::parse_volume_id($scfg->{base});
+        my $basecfg = PVE::Storage::storage_config($cfg, $baseid);
+        my $plugin = PVE::Storage::Plugin->lookup($basecfg->{type});
+        # the activation of this storage activated the base storage with the same cache before
+        $plugin->activate_volume($baseid, $basecfg, $volname, undef, $cache);
+    };
+    warn "storage '$storeid': activating base volume '$scfg->{base}' failed - $@" if $@;
+
+    # A new device invalidates LVM's hints, but a PV written remotely to an existing device does
+    # not; retry without hints after each missing-base activation.
+    $cache->{vgs} = lvm_vgs();
+    # the logical volume list of the cache can predate the volume group too
+    delete $cache->{lvs};
+    find_unhinted_vgs($cache) if !$cache->{vgs}->{ $scfg->{vgname} };
+}
+
 sub activate_storage {
     my ($class, $storeid, $scfg, $cache) = @_;
 
@@ -1614,6 +1640,10 @@ sub activate_storage {
         sub {
             $cache->{vgs} = lvm_vgs(undef, undef, $cache) if !$cache->{vgs};
 
+            # Waiting for a missing base must not stall periodic status collection.
+            activate_missing_base($storeid, $scfg, $cache)
+                if $scfg->{base} && !$cache->{status_only} && !$cache->{vgs}->{ $scfg->{vgname} };
+
             find_unhinted_vgs($cache)
                 if !$cache->{vgscaned} && !$cache->{vgs}->{ $scfg->{vgname} };
         },
diff --git a/src/test/Makefile b/src/test/Makefile
index 97b28929..97d9db4d 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -1,6 +1,6 @@
 all: test
 
-test: test_zfspoolplugin test_lvmplugin test_lvm_create test_lvm_lock_retry test_lvm_base_volume test_lvm_hints test_lvm_status test_status_command test_disklist test_disk_api test_bwlimit test_plugin test_ovf test_volume_access test_multipath test_iscsi test_lun_identity test_multipath_api test_san_api test_san_libraries
+test: test_zfspoolplugin test_lvmplugin test_lvm_create test_lvm_lock_retry test_lvm_base_volume test_lvm_hints test_lvm_base_activation test_lvm_status test_status_command test_disklist test_disk_api test_bwlimit test_plugin test_ovf test_volume_access test_multipath test_iscsi test_lun_identity test_multipath_api test_san_api test_san_libraries
 
 test_zfspoolplugin: run_test_zfspoolplugin.pl
 	./run_test_zfspoolplugin.pl
@@ -23,6 +23,9 @@ test_lvm_base_volume: run_test_lvm_base_volume.pl
 test_lvm_hints: run_test_lvm_hints.pl
 	./run_test_lvm_hints.pl
 
+test_lvm_base_activation: run_test_lvm_base_activation.pl
+	./run_test_lvm_base_activation.pl
+
 test_lvm_status: run_test_lvm_status.pl
 	./run_test_lvm_status.pl
 
diff --git a/src/test/run_test_lvm_base_activation.pl b/src/test/run_test_lvm_base_activation.pl
new file mode 100755
index 00000000..4d448163
--- /dev/null
+++ b/src/test/run_test_lvm_base_activation.pl
@@ -0,0 +1,130 @@
+#!/usr/bin/perl
+
+# Test the activation of an LVM storage whose volume group sits on a base volume of another storage.
+# If the volume group is missing, activation activates the base volume, waits for a device that is
+# still appearing, and looks for the volume group again, without the LVM device hints if needed.
+# Status-only activation skips the base volume and only lists without the hints. vgs, pvscan and the
+# base plugin 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;
+use PVE::Storage::LVMPlugin;
+
+my $CLASS = 'PVE::Storage::LVMPlugin';
+
+# the volume groups vgs lists through the device hints, those only a listing without them finds
+my ($vgs, $unhinted, @calls) = ([], []);
+
+my $lvm = Test::MockModule->new('PVE::Storage::LVMPlugin');
+$lvm->mock(
+    lvm_vgs => sub {
+        my ($includepvs, $nohints) = @_;
+        push @calls, $nohints ? 'vgs --nohints' : 'vgs';
+        return { map { $_ => {} } $vgs->@*, ($nohints ? $unhinted->@* : ()) };
+    },
+);
+my $status_command = Test::MockModule->new('PVE::Storage::StatusCommand');
+$status_command->mock(
+    run_for_cache => sub {
+        my ($cache, $cmd) = @_;
+        push @calls, join(' ', $cmd->[0] =~ s|^/sbin/||r, $cmd->@[1 .. $#$cmd]);
+    },
+);
+
+my $storage = Test::MockModule->new('PVE::Storage');
+$storage->mock(config => sub { return { ids => {} } });
+$storage->mock(storage_config => sub { return { type => 'fakebase' } });
+
+my $base_fails;
+my $plugin = Test::MockModule->new('PVE::Storage::Plugin');
+$plugin->mock(lookup => sub { return 'FakeBasePlugin' });
+
+{
+    no warnings 'once';
+    *FakeBasePlugin::activate_volume = sub {
+        my ($class, $storeid, $scfg, $volname, $snapname, $cache) = @_;
+        push @calls, "base $storeid:$volname" . ($cache->{marker} ? ' shared-cache' : '');
+        die "volume '$volname' is not available on this node\n" if $base_fails;
+        # the device appeared while waiting, and with it the volume group
+        push $vgs->@*, $volname =~ s/^vol-//r;
+    };
+}
+
+my sub activate {
+    my ($scfg, $cache, $vgs_now, $unhinted_now) = @_;
+
+    ($vgs, $unhinted, @calls) = ($vgs_now // [], $unhinted_now // []);
+    my @warnings;
+    local $SIG{__WARN__} = sub { push @warnings, $_[0] };
+    $CLASS->activate_storage('lvm', $scfg, $cache);
+    return (join(', ', @calls), \@warnings);
+}
+
+my $with_base = { vgname => 'vg1', base => 'san:vol-vg1' };
+
+my ($calls, $warnings) = activate($with_base, { vgs => { vg1 => {} } });
+is($calls, '', 'present volume group: no base activation, no scan');
+
+($calls) = activate($with_base, { status_only => 1, vgs => {} });
+is($calls, 'vgs --nohints',
+    'status collection: only the listing without hints, no base activation');
+
+($calls) = activate($with_base, { vgs => {} }, ['vg1']);
+is($calls, 'vgs', 'volume group appeared meanwhile: a fresh list, no base activation');
+
+my $cache = { marker => 1, vgs => {}, lvs => {} };
+($calls) = activate($with_base, $cache);
+is(
+    $calls,
+    'vgs, base san:vol-vg1 shared-cache, vgs',
+    'missing volume group: base activated with the shared cache, the device brings it into the list',
+);
+ok($cache->{vgs}->{vg1}, 'and the volume group list of the cache is refreshed');
+ok(!exists($cache->{lvs}), 'and its logical volume list is dropped');
+
+# the volume group another node created on a device this node had before, after an earlier probe
+$cache = { vgs => {}, vgscaned => 1 };
+{
+    no warnings 'redefine';
+    local *FakeBasePlugin::activate_volume = sub { push @calls, 'base present' };
+    ($calls) = activate($with_base, $cache, [], ['vg1']);
+}
+is(
+    $calls,
+    'vgs, base present, vgs, vgs --nohints, pvscan --cache',
+    'volume group missing from the hints: found without them despite an earlier probe',
+);
+ok($cache->{vgs}->{vg1}, 'and added to the volume group list of the cache');
+
+$cache = { vgs => {} };
+activate($with_base, $cache);
+($calls) = activate({ vgname => 'vg2', base => 'san:vol-vg2' }, $cache, ['vg1']);
+is($calls, 'vgs, base san:vol-vg2, vgs', 'a second base in the same cache is activated');
+
+$base_fails = 1;
+($calls, $warnings) = activate($with_base, { vgs => {} });
+is(
+    $calls,
+    'vgs, base san:vol-vg1, vgs, vgs --nohints',
+    'a failed base activation still looks for the volume group',
+);
+like(
+    $warnings->[0] // '',
+    qr/^storage 'lvm': activating base volume 'san:vol-vg1' failed - volume/,
+    'and warns about it',
+);
+$base_fails = 0;
+
+$cache = { vgs => {} };
+($calls) = activate({ vgname => 'vg1' }, $cache);
+is($calls, 'vgs --nohints', 'without base: one listing without hints per cache');
+($calls) = activate({ vgname => 'vg3' }, $cache);
+is($calls, '', 'and not again for the next storage');
+
+done_testing();
-- 
2.47.3