[PATCH storage v2] fix #8211: disks: detect SAS hard disks as hdd

[Lucas Barros <lucasbarros.kam@gmail.com>]

SAS hard disks were listed with type 'unknown' and rpm -1. For a
rotational device the type 'hdd' is only set if a rotation rate is
known, and that rate is only read from the ID_ATA_ROTATION_RATE_RPM
udev property, which only exists for ATA devices.

SCSI devices report their nominal rotation rate in the block device
characteristics VPD page (0xb1), which the kernel exposes as
device/vpd_pgb1 in sysfs. Use that as a fallback if udev does not
provide a rotation rate, so SAS hard disks are listed as 'hdd' with
their actual rpm. Devices without that page, or which do not report a
rotation rate there, keep the current behavior.

Add a test case for a SAS hard disk with such a VPD page.

Signed-off-by: Lucas Barros <lucasbarros.kam@gmail.com>
---
Changes since v1:
* run proxmox-perltidy, which wraps the new line in get_disks(); no
  functional change

Tested on a PVE 9 test node with the full test suite of the repo
('make test' in src: 562 tests, disk list tests 7 -> 8, the new
sas_hdd case fails without the change).

No SAS hardware at hand, so I checked the real kernel interface with a
SCSI disk emulated by QEMU (virtio-scsi, scsi-hd with
rotation_rate=10000) in a VM with kernel 6.12, which exposes the page
in sysfs and has no ATA rotation rate in udev:

  $ od -An -tx1 -N8 /sys/block/sdb/device/vpd_pgb1
   00 b1 00 3c 27 10 00 00
  $ cat /sys/block/sdb/queue/rotational
  1

Running get_disks() against that sysfs and udevadm data:

  without the patch: 'type' => 'unknown', 'rpm' => -1
  with the patch:    'type' => 'hdd',     'rpm' => 10000

The VPD page layout is from SBC (Block Device Characteristics VPD
page, medium rotation rate in bytes 4-5). The fixture contains a small
binary file (the raw VPD page).

This patch was prepared with the help of an LLM based coding assistant;
I reviewed the change and ran the tests mentioned above myself.

 src/PVE/Diskmanage.pm                         |  14 ++++++++
 src/test/disk_tests/sas_hdd/disklist          |   1 +
 .../disk_tests/sas_hdd/disklist_expected.json |  18 ++++++++++
 src/test/disk_tests/sas_hdd/sda/device/model  |   1 +
 src/test/disk_tests/sas_hdd/sda/device/vendor |   1 +
 .../disk_tests/sas_hdd/sda/device/vpd_pgb1    | Bin 0 -> 64 bytes
 .../disk_tests/sas_hdd/sda/queue/rotational   |   1 +
 src/test/disk_tests/sas_hdd/sda/size          |   1 +
 src/test/disk_tests/sas_hdd/sda_udevadm       |  31 ++++++++++++++++++
 9 files changed, 68 insertions(+)
 create mode 100644 src/test/disk_tests/sas_hdd/disklist
 create mode 100644 src/test/disk_tests/sas_hdd/disklist_expected.json
 create mode 100644 src/test/disk_tests/sas_hdd/sda/device/model
 create mode 100644 src/test/disk_tests/sas_hdd/sda/device/vendor
 create mode 100644 src/test/disk_tests/sas_hdd/sda/device/vpd_pgb1
 create mode 100644 src/test/disk_tests/sas_hdd/sda/queue/rotational
 create mode 100644 src/test/disk_tests/sas_hdd/sda/size
 create mode 100644 src/test/disk_tests/sas_hdd/sda_udevadm

diff --git a/src/PVE/Diskmanage.pm b/src/PVE/Diskmanage.pm
index d5bd147..46abd98 100644
--- a/src/PVE/Diskmanage.pm
+++ b/src/PVE/Diskmanage.pm
@@ -416,6 +416,18 @@ sub get_sysdir_info {
         $data->{$attr} = trim(file_read_firstline("$sysdir/device/$attr")) || 'unknown';
     }
 
+    # SCSI devices do not have the ATA rotation rate in udev, but report it in
+    # bytes 4-5 of the block device characteristics VPD page (0xb1), where
+    # values from 0x0401 to 0xfffe are the nominal rotation rate in rpm
+    my $vpd_pgb1 = "$sysdir/device/vpd_pgb1";
+    if (-f $vpd_pgb1) {
+        my $raw = eval { file_get_contents($vpd_pgb1, 4096) } // '';
+        if (length($raw) >= 6) {
+            my $rate = unpack('n', substr($raw, 4, 2));
+            $data->{rpm} = $rate if $rate >= 0x0401 && $rate <= 0xfffe;
+        }
+    }
+
     return $data;
 }
 
@@ -587,6 +599,8 @@ sub get_disks {
                 $type = 'nvme' if $dev =~ m/^nvme\d+n\d+$/;
                 $data->{rpm} = 0;
             } elsif ($sysdata->{rotational} == 1) {
+                $data->{rpm} = $sysdata->{rpm}
+                    if $data->{rpm} == -1 && defined($sysdata->{rpm});
                 if ($data->{rpm} != -1) {
                     $type = 'hdd';
                 } elsif ($data->{usb}) {
diff --git a/src/test/disk_tests/sas_hdd/disklist b/src/test/disk_tests/sas_hdd/disklist
new file mode 100644
index 0000000..9191c61
--- /dev/null
+++ b/src/test/disk_tests/sas_hdd/disklist
@@ -0,0 +1 @@
+sda
diff --git a/src/test/disk_tests/sas_hdd/disklist_expected.json b/src/test/disk_tests/sas_hdd/disklist_expected.json
new file mode 100644
index 0000000..6492219
--- /dev/null
+++ b/src/test/disk_tests/sas_hdd/disklist_expected.json
@@ -0,0 +1,18 @@
+{
+    "sda" : {
+	"gpt" : 1,
+	"devpath" : "/dev/sda",
+	"type" : "hdd",
+	"model" : "MODEL1",
+	"health" : "UNKNOWN",
+	"osdid" : -1,
+	"osdid-list" : null,
+	"wwn" : "0x0000000000000000",
+	"vendor" : "VENDOR1",
+	"rpm" : 10000,
+	"size" : 5120000,
+	"serial" : "SER2",
+	"wearout" : "N/A",
+	"by_id_link" : "/dev/disk/by-id/scsi-00000000000000000"
+    }
+}
diff --git a/src/test/disk_tests/sas_hdd/sda/device/model b/src/test/disk_tests/sas_hdd/sda/device/model
new file mode 100644
index 0000000..6b69674
--- /dev/null
+++ b/src/test/disk_tests/sas_hdd/sda/device/model
@@ -0,0 +1 @@
+MODEL1
diff --git a/src/test/disk_tests/sas_hdd/sda/device/vendor b/src/test/disk_tests/sas_hdd/sda/device/vendor
new file mode 100644
index 0000000..a7894eb
--- /dev/null
+++ b/src/test/disk_tests/sas_hdd/sda/device/vendor
@@ -0,0 +1 @@
+VENDOR1
diff --git a/src/test/disk_tests/sas_hdd/sda/device/vpd_pgb1 b/src/test/disk_tests/sas_hdd/sda/device/vpd_pgb1
new file mode 100644
index 0000000000000000000000000000000000000000..3dbd0c4aa8fc30d5e1c671feca4e93b4b368a342
GIT binary patch
literal 64
QcmZSB$Y7%`z(6(t07mZtB>(^b

literal 0
HcmV?d00001

diff --git a/src/test/disk_tests/sas_hdd/sda/queue/rotational b/src/test/disk_tests/sas_hdd/sda/queue/rotational
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/src/test/disk_tests/sas_hdd/sda/queue/rotational
@@ -0,0 +1 @@
+1
diff --git a/src/test/disk_tests/sas_hdd/sda/size b/src/test/disk_tests/sas_hdd/sda/size
new file mode 100644
index 0000000..5caff40
--- /dev/null
+++ b/src/test/disk_tests/sas_hdd/sda/size
@@ -0,0 +1 @@
+10000
diff --git a/src/test/disk_tests/sas_hdd/sda_udevadm b/src/test/disk_tests/sas_hdd/sda_udevadm
new file mode 100644
index 0000000..ac0744d
--- /dev/null
+++ b/src/test/disk_tests/sas_hdd/sda_udevadm
@@ -0,0 +1,31 @@
+P: /devices/pci0000:00/0000:00:03.0/0000:02:00.0/host4/port-4:0/end_device-4:0/target4:0:0/4:0:0:0/block/sda
+N: sda
+S: disk/by-id/scsi-00000000000000000
+S: disk/by-id/wwn-0x0000000000000000
+S: disk/by-path/pci-0000:02:00.0-sas-0x0000000000000000-lun-0
+E: DEVLINKS=/dev/disk/by-id/scsi-00000000000000000 /dev/disk/by-id/wwn-0x0000000000000000 /dev/disk/by-path/pci-0000:02:00.0-sas-0x0000000000000000-lun-0
+E: DEVNAME=/dev/sda
+E: DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:02:00.0/host4/port-4:0/end_device-4:0/target4:0:0/4:0:0:0/block/sda
+E: DEVTYPE=disk
+E: ID_BUS=scsi
+E: ID_MODEL=MODEL1
+E: ID_MODEL_ENC=MODEL1\x20\x20\x20\x20\x20\x20
+E: ID_PART_TABLE_TYPE=gpt
+E: ID_PART_TABLE_UUID=605740f0-44a1-4dc5-9fea-bde166df963e
+E: ID_PATH=pci-0000:02:00.0-sas-0x0000000000000000-lun-0
+E: ID_PATH_TAG=pci-0000_02_00_0-sas-0x0000000000000000-lun-0
+E: ID_REVISION=ES64
+E: ID_SCSI=1
+E: ID_SCSI_SERIAL=SERIAL
+E: ID_SERIAL=SERIAL2
+E: ID_SERIAL_SHORT=SER2
+E: ID_TYPE=disk
+E: ID_VENDOR=VENDOR1
+E: ID_VENDOR_ENC=VENDOR1\x20
+E: ID_WWN=0x0000000000000000
+E: ID_WWN_WITH_EXTENSION=0x0000000000000000
+E: MAJOR=8
+E: MINOR=0
+E: SUBSYSTEM=block
+E: TAGS=:systemd:
+E: USEC_INITIALIZED=667541
-- 
2.47.3