[PATCH v3 storage 24/76] api: scan: san-luns: report path state

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

From: Dietmar Maurer <dietmar@proxmox.com>

A failed path stays in the device-mapper table and sysfs slave list
until the transport removes it, so counting slaves cannot detect a
degraded map. Report active path counts, fault counters, and per-path
states from multipathd instead. Device-mapper active paths count,
including ALUA standby paths; failed and undefined paths do not.

For native NVMe multipath, report controller and ANA states from sysfs.
Queries are best-effort: failures leave the optional fields absent.

Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
[TL: use the shared parser of PVE::Multipath instead of the helper and
 parser of the RFC commit 'diskmanage: add helper querying multipath
 path state', which is dropped; a path whose state multipathd reports as
 undefined no longer counts as active; name the active path count
 paths-active, like the total]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 src/PVE/API2/Storage/Scan.pm                  |  72 +++++++++-
 src/PVE/CLI/pvesm.pm                          |   3 +-
 src/PVE/Storage/SANScan.pm                    |  32 ++++-
 src/test/disk_tests/multipath/multipathd      | 124 ++++++++++++++++++
 .../multipath/multipathd_status_expected.json |  42 ++++++
 src/test/disklist_test.pm                     |  18 +++
 src/test/run_san_api_tests.pl                 |  12 +-
 7 files changed, 297 insertions(+), 6 deletions(-)
 create mode 100644 src/test/disk_tests/multipath/multipathd
 create mode 100644 src/test/disk_tests/multipath/multipathd_status_expected.json

diff --git a/src/PVE/API2/Storage/Scan.pm b/src/PVE/API2/Storage/Scan.pm
index 58570f46..73632a1f 100644
--- a/src/PVE/API2/Storage/Scan.pm
+++ b/src/PVE/API2/Storage/Scan.pm
@@ -381,7 +381,9 @@ __PACKAGE__->register_method({
     path => 'san-luns',
     method => 'GET',
     description => "List block devices (disks and multipath devices) together with their"
-        . " usage, for example as candidates for a SAN backed LVM volume group.",
+        . " usage, for example as candidates for a SAN backed LVM volume group. For multipath"
+        . " mapped devices and NVMe namespaces handled by native NVMe multipath, the state"
+        . " of the member paths is included.",
     protected => 1,
     proxyto => "node",
     permissions => {
@@ -424,10 +426,76 @@ __PACKAGE__->register_method({
                 serial => { type => 'string', optional => 1 },
                 wwn => { type => 'string', optional => 1 },
                 'paths-total' => {
-                    description => "The number of paths of a multipath mapped device.",
+                    description => "The number of paths of a multipath mapped device or of"
+                        . " an NVMe namespace attached through multiple controllers.",
                     type => 'integer',
                     optional => 1,
                 },
+                'paths-active' => {
+                    description => "The number of paths that are usable, that is, neither"
+                        . " reported as failed by multipathd nor on a dead controller or"
+                        . " inaccessible through ANA.",
+                    type => 'integer',
+                    optional => 1,
+                },
+                'path-faults' => {
+                    description => "The number of path failures of a multipath mapped"
+                        . " device since its creation, as reported by multipathd.",
+                    type => 'integer',
+                    optional => 1,
+                },
+                'path-list' => {
+                    description => "The member paths of the device with their state.",
+                    type => 'array',
+                    optional => 1,
+                    items => {
+                        type => 'object',
+                        properties => {
+                            device => {
+                                description => "The path block device, or the NVMe"
+                                    . " controller device the path uses.",
+                                type => 'string',
+                            },
+                            state => {
+                                description => "The device mapper state of the path (for"
+                                    . " example active or failed) or the state of the NVMe"
+                                    . " controller (for example live or connecting).",
+                                type => 'string',
+                            },
+                            'checker-state' => {
+                                description => "The state reported by the multipathd path"
+                                    . " checker, for example ready, ghost or faulty.",
+                                type => 'string',
+                                optional => 1,
+                            },
+                            'ana-state' => {
+                                description => "The Asymmetric Namespace Access state of"
+                                    . " the namespace on this path, for example optimized"
+                                    . " or inaccessible.",
+                                type => 'string',
+                                optional => 1,
+                            },
+                            'host-wwpn' => {
+                                description => "The WWPN of the host (initiator) port of"
+                                    . " Fibre Channel attached paths.",
+                                type => 'string',
+                                optional => 1,
+                            },
+                            'target-wwpn' => {
+                                description => "The WWPN of the target port of Fibre"
+                                    . " Channel attached paths.",
+                                type => 'string',
+                                optional => 1,
+                            },
+                            address => {
+                                description => "The address of the NVMe controller the"
+                                    . " path uses.",
+                                type => 'string',
+                                optional => 1,
+                            },
+                        },
+                    },
+                },
                 usage => {
                     description => "How the device is currently used. 'lvm' means it is an LVM"
                         . " physical volume, see the 'vgname' property.",
diff --git a/src/PVE/CLI/pvesm.pm b/src/PVE/CLI/pvesm.pm
index 501b7b61..67419da2 100755
--- a/src/PVE/CLI/pvesm.pm
+++ b/src/PVE/CLI/pvesm.pm
@@ -743,7 +743,8 @@ our $cmddef = {
             { node => $nodename },
             sub {
                 my ($data, $schema, $options) = @_;
-                my $columns = [qw(devpath size transport usage vgname used-by-storage)];
+                my $columns =
+                    [qw(devpath size transport paths-active usage vgname used-by-storage)];
                 PVE::CLIFormatter::print_api_result($data, $schema, $columns, $options);
             },
             $PVE::RESTHandler::standard_output_options,
diff --git a/src/PVE/Storage/SANScan.pm b/src/PVE/Storage/SANScan.pm
index 6e968f65..ee311cb9 100644
--- a/src/PVE/Storage/SANScan.pm
+++ b/src/PVE/Storage/SANScan.pm
@@ -3,6 +3,7 @@ package PVE::Storage::SANScan;
 use v5.36;
 
 use PVE::Diskmanage;
+use PVE::Multipath;
 use PVE::Storage;
 use PVE::Storage::ISCSIPlugin;
 use PVE::Storage::LVMPlugin;
@@ -11,6 +12,15 @@ my sub collect_inventory($cfg, $node) {
     my $disks = PVE::Diskmanage::get_disks(undef, 1, 0);
     my $multipath = PVE::Diskmanage::get_multipath_disks();
 
+    # Path state is auxiliary information; a multipathd failure must not break the scan.
+    my $mp_status = {};
+    if (scalar(keys %$multipath)) {
+        $mp_status = eval { PVE::Multipath::path_status_by_wwid() } // {};
+        warn $@ if $@;
+    }
+    my $nvme_status = eval { PVE::Diskmanage::get_nvme_path_status() } // {};
+    warn $@ if $@;
+
     # map PV device to VG name, PVs may sit on a partition of a listed device
     my $vgs = PVE::Storage::LVMPlugin::lvm_vgs(1);
     my $pv2vg = {};
@@ -63,6 +73,8 @@ my sub collect_inventory($cfg, $node) {
     return {
         disks => $disks,
         multipath => $multipath,
+        mp_status => $mp_status,
+        nvme_status => $nvme_status,
         vg_used_by => $vg_used_by,
         iscsi_target_used_by => $iscsi_target_used_by,
         find_vg => $find_vg,
@@ -71,8 +83,9 @@ my sub collect_inventory($cfg, $node) {
 }
 
 my sub build_lun_rows($cfg, $inventory) {
-    my ($disks, $multipath, $vg_used_by, $find_vg, $iscsi_used_by) =
-        $inventory->@{qw(disks multipath vg_used_by find_vg iscsi_used_by)};
+    my ($disks, $multipath, $mp_status, $nvme_status, $vg_used_by, $find_vg, $iscsi_used_by) =
+        $inventory->@{ qw(disks multipath mp_status nvme_status vg_used_by find_vg
+            iscsi_used_by) };
 
     my $records = [];
     my $add_entry = sub($disk) {
@@ -84,6 +97,21 @@ my sub build_lun_rows($cfg, $inventory) {
             $entry->{$key} = $value if defined($value) && $value ne 'unknown';
         }
 
+        # aggregated multipath entries carry their member list and their WWID as wwn
+        if (defined($disk->{slaves})) {
+            if (my $status = $mp_status->{ $disk->{wwn} // '' }) {
+                $entry->{'paths-active'} = $status->{active};
+                $entry->{'path-faults'} = $status->{faults};
+                $entry->{'path-list'} = $status->{paths};
+            }
+        } elsif ($disk->{devpath} =~ m|^/dev/(nvme\d+n\d+)$|) {
+            if (my $status = $nvme_status->{$1}) {
+                $entry->{'paths-total'} = scalar($status->{paths}->@*);
+                $entry->{'paths-active'} = $status->{active};
+                $entry->{'path-list'} = $status->{paths};
+            }
+        }
+
         if (!defined($disk->{used})) {
             $entry->{usage} = 'unused';
         } elsif ($disk->{used} eq 'LVM') {
diff --git a/src/test/disk_tests/multipath/multipathd b/src/test/disk_tests/multipath/multipathd
new file mode 100644
index 00000000..40243f91
--- /dev/null
+++ b/src/test/disk_tests/multipath/multipathd
@@ -0,0 +1,124 @@
+{
+   "major_version": 0,
+   "minor_version": 1,
+   "maps": [{
+      "name" : "mpatha",
+      "uuid" : "3600140500000000000000000000000001",
+      "sysfs" : "dm-0",
+      "failback" : "immediate",
+      "queueing" : "5 chk",
+      "paths" : 2,
+      "write_prot" : "rw",
+      "dm_st" : "active",
+      "features" : "1 queue_if_no_path",
+      "hwhandler" : "1 alua",
+      "action" : "",
+      "path_faults" : 0,
+      "vend" : "SANVEND ",
+      "prod" : "SANMODEL",
+      "rev" : "4.1",
+      "switch_grp" : 0,
+      "map_loads" : 1,
+      "total_q_time" : 0,
+      "q_timeouts" : 0,
+      "path_groups": [{
+         "selector" : "service-time 0",
+         "pri" : 50,
+         "dm_st" : "active",
+         "marginal_st" : "normal",
+         "group" : 1,
+         "paths": [{
+            "dev" : "sda",
+            "dev_t" : "8:0",
+            "dm_st" : "active",
+            "dev_st" : "running",
+            "chk_st" : "ready",
+            "checker" : "tur",
+            "pri" : 50,
+            "host_wwnn" : "0x20000024ff7d6a1c",
+            "target_wwnn" : "0x2000d039ea28bf00",
+            "host_wwpn" : "0x21000024ff7d6a1c",
+            "target_wwpn" : "0x2100d039ea28bf00",
+            "host_adapter" : "0000:41:00.0",
+            "lun_hex" : "0x0001000000000000",
+            "marginal_st" : "normal"
+         },
+         {
+            "dev" : "sdb",
+            "dev_t" : "8:16",
+            "dm_st" : "active",
+            "dev_st" : "running",
+            "chk_st" : "ready",
+            "checker" : "tur",
+            "pri" : 50,
+            "host_wwnn" : "0x20000024ff7d6a1d",
+            "target_wwnn" : "0x2000d039ea28bf00",
+            "host_wwpn" : "0x21000024ff7d6a1d",
+            "target_wwpn" : "0x2200d039ea28bf00",
+            "host_adapter" : "0000:41:00.1",
+            "lun_hex" : "0x0001000000000000",
+            "marginal_st" : "normal"
+         }]
+      }]
+   },
+   {
+      "name" : "mpathb",
+      "uuid" : "3600140500000000000000000000000002",
+      "sysfs" : "dm-1",
+      "failback" : "immediate",
+      "queueing" : "5 chk",
+      "paths" : 2,
+      "write_prot" : "rw",
+      "dm_st" : "active",
+      "features" : "1 queue_if_no_path",
+      "hwhandler" : "1 alua",
+      "action" : "",
+      "path_faults" : 3,
+      "vend" : "SANVEND ",
+      "prod" : "SANMODEL",
+      "rev" : "4.1",
+      "switch_grp" : 0,
+      "map_loads" : 1,
+      "total_q_time" : 0,
+      "q_timeouts" : 0,
+      "path_groups": [{
+         "selector" : "service-time 0",
+         "pri" : 50,
+         "dm_st" : "active",
+         "marginal_st" : "normal",
+         "group" : 1,
+         "paths": [{
+            "dev" : "sdc",
+            "dev_t" : "8:32",
+            "dm_st" : "active",
+            "dev_st" : "running",
+            "chk_st" : "ready",
+            "checker" : "tur",
+            "pri" : 50,
+            "host_wwnn" : "0x20000024ff7d6a1c",
+            "target_wwnn" : "0x2000d039ea28bf00",
+            "host_wwpn" : "0x21000024ff7d6a1c",
+            "target_wwpn" : "0x2100d039ea28bf00",
+            "host_adapter" : "0000:41:00.0",
+            "lun_hex" : "0x0002000000000000",
+            "marginal_st" : "normal"
+         },
+         {
+            "dev" : "sdd",
+            "dev_t" : "8:48",
+            "dm_st" : "failed",
+            "dev_st" : "running",
+            "chk_st" : "faulty",
+            "checker" : "tur",
+            "pri" : 0,
+            "host_wwnn" : "0x20000024ff7d6a1d",
+            "target_wwnn" : "0x2000d039ea28bf00",
+            "host_wwpn" : "0x21000024ff7d6a1d",
+            "target_wwpn" : "0x2200d039ea28bf00",
+            "host_adapter" : "0000:41:00.1",
+            "lun_hex" : "0x0002000000000000",
+            "marginal_st" : "normal"
+         }]
+      }]
+   }]
+}
diff --git a/src/test/disk_tests/multipath/multipathd_status_expected.json b/src/test/disk_tests/multipath/multipathd_status_expected.json
new file mode 100644
index 00000000..d57e620d
--- /dev/null
+++ b/src/test/disk_tests/multipath/multipathd_status_expected.json
@@ -0,0 +1,42 @@
+{
+    "3600140500000000000000000000000001": {
+        "active": 2,
+        "faults": 0,
+        "paths": [
+            {
+                "device": "/dev/sda",
+                "state": "active",
+                "checker-state": "ready",
+                "host-wwpn": "0x21000024ff7d6a1c",
+                "target-wwpn": "0x2100d039ea28bf00"
+            },
+            {
+                "device": "/dev/sdb",
+                "state": "active",
+                "checker-state": "ready",
+                "host-wwpn": "0x21000024ff7d6a1d",
+                "target-wwpn": "0x2200d039ea28bf00"
+            }
+        ]
+    },
+    "3600140500000000000000000000000002": {
+        "active": 1,
+        "faults": 3,
+        "paths": [
+            {
+                "device": "/dev/sdc",
+                "state": "active",
+                "checker-state": "ready",
+                "host-wwpn": "0x21000024ff7d6a1c",
+                "target-wwpn": "0x2100d039ea28bf00"
+            },
+            {
+                "device": "/dev/sdd",
+                "state": "failed",
+                "checker-state": "faulty",
+                "host-wwpn": "0x21000024ff7d6a1d",
+                "target-wwpn": "0x2200d039ea28bf00"
+            }
+        ]
+    }
+}
diff --git a/src/test/disklist_test.pm b/src/test/disklist_test.pm
index c6fb620c..ae072fb2 100644
--- a/src/test/disklist_test.pm
+++ b/src/test/disklist_test.pm
@@ -6,6 +6,7 @@ use warnings;
 use lib qw(..);
 
 use PVE::Diskmanage;
+use PVE::Multipath;
 use PVE::Tools;
 
 use Test::MockModule;
@@ -16,6 +17,7 @@ use Data::Dumper;
 my $testcasedir; # current case directory
 my $testcount = 0; # testcount for TAP::Harness
 my $diskmanage_module; # mockmodule for PVE::Diskmanage
+my $multipath_module; # mockmodule for PVE::Multipath
 my $print = 0;
 
 my $multipath_aliases = {
@@ -68,6 +70,8 @@ sub mocked_run_command {
                 $content = '{}';
             }
             @$outputlines = split(/\n/, $content);
+        } elsif ($cmd->[0] =~ m/multipathd/i) {
+            @$outputlines = split(/\n/, read_test_file('multipathd'));
         } else {
             die "unexpected run_command call: '@$cmd', aborting\n";
         }
@@ -274,6 +278,16 @@ sub test_disk_list {
             is($@, '', 'the SAN opt-in accepts an unused iSCSI LUN');
         }
 
+        if (-f "disk_tests/$testcasedir/multipathd_status_expected.json") {
+            my $status = PVE::Multipath::path_status_by_wwid();
+            my $expected_status =
+                decode_json(read_test_file('multipathd_status_expected.json'));
+
+            print Dumper($status) if $print;
+            $testcount++;
+            is_deeply($status, $expected_status, 'multipath status should be the same');
+        }
+
         if (-f "disk_tests/$testcasedir/nvme_path_status_expected.json") {
             my $status = PVE::Diskmanage::get_nvme_path_status();
             my $expected_status = decode_json(read_test_file('nvme_path_status_expected.json'));
@@ -305,6 +319,10 @@ $diskmanage_module->mock('get_sysdir_size' => \&mocked_get_sysdir_size);
 print("\tMocked get_sysdir_size\n");
 $diskmanage_module->mock('assert_blockdev' => sub { return 1; });
 print("\tMocked assert_blockdev\n");
+$multipath_module = Test::MockModule->new('PVE::Multipath', no_auto => 1);
+$multipath_module->mock('run_command' => \&mocked_run_command);
+$multipath_module->mock('is_supported' => sub { return 1; });
+print("\tMocked PVE::Multipath run_command and is_supported\n");
 $diskmanage_module->mock(
     'dir_is_empty' => sub {
         my $val = shift;
diff --git a/src/test/run_san_api_tests.pl b/src/test/run_san_api_tests.pl
index de35217f..ac5a8be3 100755
--- a/src/test/run_san_api_tests.pl
+++ b/src/test/run_san_api_tests.pl
@@ -39,7 +39,7 @@ subtest 'SAN CLI columns leave machine output complete' => sub {
             is($_[1], $schema, 'complete schema forwarded');
             is_deeply(
                 $_[2],
-                [qw(devpath size transport usage vgname used-by-storage)],
+                [qw(devpath size transport paths-active usage vgname used-by-storage)],
                 'concise text columns',
             );
             is($_[3]->{'output-format'}, 'json', 'format option unchanged');
@@ -72,12 +72,22 @@ subtest 'SAN LUN path counts' => sub {
     );
     $lvm->mock(lvm_vgs => sub { {} });
     $storage->mock(config => sub { { ids => {} } });
+    # native NVMe multipath: two controllers, one of them usable
+    $disks->{'/dev/nvme0n1'} = { devpath => '/dev/nvme0n1', size => 1024 };
+    my $paths = [{ device => '/dev/nvme0' }, { device => '/dev/nvme1' }];
+    $disk->mock(
+        get_nvme_path_status => sub { { nvme0n1 => { active => 1, paths => $paths } } });
+    my $multipath = Test::MockModule->new('PVE::Multipath');
+    $multipath->mock(path_status_by_wwid => sub { {} });
     my $declared =
         PVE::API2::Storage::Scan->map_method_by_name('sanlunscan')->{returns}->{items}
         ->{properties};
     my $rows = PVE::API2::Storage::Scan->sanlunscan({ node => 'test' });
     my $by_dev = { map { ($_->{devpath} => $_) } @$rows };
     is($by_dev->{'/dev/mapper/m0'}->{'paths-total'}, 2, 'map path count');
+    # native NVMe multipath
+    is($by_dev->{'/dev/nvme0n1'}->{'paths-total'}, 2, 'namespace path count');
+    is($by_dev->{'/dev/nvme0n1'}->{'paths-active'}, 1, 'namespace usable paths');
     for my $row (@$rows) {
         my @undeclared = grep { !$declared->{$_} } sort keys %$row;
         is_deeply(\@undeclared, [], "$row->{devpath}: every field declared");
-- 
2.47.3