[pve-devel] [PATCH v2 storage] Fix #2020: use /sys to map nvmeXnY to nvmeX

[Wolfgang Bumiller]

`nvmeX` devices nodes are apparently allocated independently
from their namespace block devices `nvmeXnY` and therefore
they are not strictly related by name. For instance:
$ readlink /sys/block/nvme0n1/device
../../nvme1
$ readlink /sys/block/nvme1n1/device
../../nvme0

Here /dev/nvme0n1 is the first namespace of /dev/nvme1 while
/dev/nvme1n1 is the first namespace of /dev/nvme0.

Signed-off-by: Wolfgang Bumiller <***@proxmox.com>
---
Changes to v1:
- changed untaint regex to a more readable one.
- improved commit message

Previous comment:
I used get_sysdir_info() and made it read the `(...)/device` symlink,
otherwise I'd have to either add another helper for just that or
figure out some other way to get the testsuite to work again...

PVE/Diskmanage.pm | 12 ++++++++++--
test/disk_tests/nvme_smart/{nvme0n1/device => nvme0}/model | 0
test/disk_tests/nvme_smart/nvme0n1/device | 1 +
3 files changed, 11 insertions(+), 2 deletions(-)
rename test/disk_tests/nvme_smart/{nvme0n1/device => nvme0}/model (100%)
create mode 120000 test/disk_tests/nvme_smart/nvme0n1/device

diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index def0791..854f0a5 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -79,8 +79,11 @@ sub get_smart_data {

my $returncode = 0;

- $disk =~ s/n\d+$//
- if $disk =~ m!^/dev/nvme\d+n\d+$!;
+ if ($disk =~ m!^/dev/(nvme\d+n\d+)$!) {
+ my $info = get_sysdir_info("/sys/block/$1");
+ $disk = "/dev/".($info->{device}
+ or die "failed to get nvme controller device for $disk\n");
+ }

my $cmd = [$SMARTCTL, '-H'];
push @$cmd, '-A', '-f', 'brief' if !$healthonly;
@@ -306,6 +309,11 @@ sub get_sysdir_info {
$data->{vendor} = file_read_firstline("$sysdir/device/vendor") || 'unknown';
$data->{model} = file_read_firstline("$sysdir/device/model") || 'unknown';

+ if (defined(my $device = readlink("$sysdir/device"))) {
+ # strip directory and untaint:
+ ($data->{device}) = $device =~ m!([^/]+)$!;
+ }
+
return $data;
}

diff --git a/test/disk_tests/nvme_smart/nvme0n1/device/model b/test/disk_tests/nvme_smart/nvme0/model
similarity index 100%
rename from test/disk_tests/nvme_smart/nvme0n1/device/model
rename to test/disk_tests/nvme_smart/nvme0/model
diff --git a/test/disk_tests/nvme_smart/nvme0n1/device b/test/disk_tests/nvme_smart/nvme0n1/device
new file mode 120000
index 0000000..e890f3e
--- /dev/null
+++ b/test/disk_tests/nvme_smart/nvme0n1/device
@@ -0,0 +1 @@
+../nvme0
\ No newline at end of file

[Thomas Lamprecht]

Post by Wolfgang Bumiller
`nvmeX` devices nodes are apparently allocated independently
from their namespace block devices `nvmeXnY` and therefore
$ readlink /sys/block/nvme0n1/device
../../nvme1
$ readlink /sys/block/nvme1n1/device
../../nvme0
Here /dev/nvme0n1 is the first namespace of /dev/nvme1 while
/dev/nvme1n1 is the first namespace of /dev/nvme0.
---
- changed untaint regex to a more readable one.
- improved commit message

applied, thanks!

Post by Wolfgang Bumiller
I used get_sysdir_info() and made it read the `(...)/device` symlink,
otherwise I'd have to either add another helper for just that or
figure out some other way to get the testsuite to work again...
PVE/Diskmanage.pm | 12 ++++++++++--
test/disk_tests/nvme_smart/{nvme0n1/device => nvme0}/model | 0
test/disk_tests/nvme_smart/nvme0n1/device | 1 +
3 files changed, 11 insertions(+), 2 deletions(-)
rename test/disk_tests/nvme_smart/{nvme0n1/device => nvme0}/model (100%)
create mode 120000 test/disk_tests/nvme_smart/nvme0n1/device
diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index def0791..854f0a5 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -79,8 +79,11 @@ sub get_smart_data {
my $returncode = 0;
- $disk =~ s/n\d+$//
- if $disk =~ m!^/dev/nvme\d+n\d+$!;
+ if ($disk =~ m!^/dev/(nvme\d+n\d+)$!) {
+ my $info = get_sysdir_info("/sys/block/$1");
+ $disk = "/dev/".($info->{device}
+ or die "failed to get nvme controller device for $disk\n");
+ }
my $cmd = [$SMARTCTL, '-H'];
@@ -306,6 +309,11 @@ sub get_sysdir_info {
$data->{vendor} = file_read_firstline("$sysdir/device/vendor") || 'unknown';
$data->{model} = file_read_firstline("$sysdir/device/model") || 'unknown';
+ if (defined(my $device = readlink("$sysdir/device"))) {
+ ($data->{device}) = $device =~ m!([^/]+)$!;
+ }
+
return $data;
}
diff --git a/test/disk_tests/nvme_smart/nvme0n1/device/model b/test/disk_tests/nvme_smart/nvme0/model
similarity index 100%
rename from test/disk_tests/nvme_smart/nvme0n1/device/model
rename to test/disk_tests/nvme_smart/nvme0/model
diff --git a/test/disk_tests/nvme_smart/nvme0n1/device b/test/disk_tests/nvme_smart/nvme0n1/device
new file mode 120000
index 0000000..e890f3e
--- /dev/null
+++ b/test/disk_tests/nvme_smart/nvme0n1/device
@@ -0,0 +1 @@
+../nvme0
\ No newline at end of file