Merge ~rodsmith/plainbox-provider-checkbox:update-disk-read-performance-thresholds into plainbox-provider-checkbox:master

[Rod Smith]

Update disk_read_performance_test with new device types & more modern minimum acceptable speeds.

Added device types for NVDIMM, SSD, ATA, & SCSI devices -- the latter two were identified by the script but fell back to the default (15 MB/s) pass criterion, and the first two require new code to identify.

This MR also sets new or updates pass criteria for NVMe, NVDIMM, MDADM, ATA, SCSI, and SSD devices, based on data from over 50 devices in a semi-random sample -- see:

https://docs.google.com/spreadsheets/d/183H8rf0YpmpjI4OVr9pjG-CQvoBkWqE7qCsiwtduuiQ/edit#gid=0

Criteria were set a little lower than the lowest observed value for most devices, with an exception being HDD (ATA and SCSI) devices, which were set above the pathetic level of a couple of very old disks and a bit below the lowest observed value for more modern hardware. I set the NVDIMM value to match the NVMe value because I have access to just one machine with NVDIMM hardware, and average performance levels seemed similar.

[Jeff Lane  (bladernr)]

LGTM. I'd suggest at least letting Devices know this changed, but it'll sit in Dev for a while and they can test it and change if necessary.

review: Approve

[Jonathan Cave (jocave)]

Looking at the git log of this file there have been a number of commits over the past 6 months or so that modify these values one way or the other, hence I have some reservations that the values are being "experimented" with.

However, your spreadsheet is the most authoritative source I have seen for identifying reasonable limits which makes me think this should be ok.

The only other suggestion I can think of would be to somehow modify the script/tests such that there was a job with an easily parse-able value for the speed (no other output for example) such that a script to parse C3 could be written and you wouldnt need to manually produce such a spreadsheet again. Just an idea.

review: Approve

[Rod Smith (rodsmith)]

Thanks, Jonathan. I've fired off an e-mail to Maciej and Jonathan, who have made the most recent commits altering these values, so as to solicit their input.

Your suggestion to create output that's easily parsed from C3 by an outside script is a good one, but I think that's a task for another commit. I'll give it some thought.

[Rod Smith (rodsmith)]

s/e-mail to Maciej and Jonathan/e-mail to Maciej and Sylvain/

...although I did cc: you, Jonathan.

[Sylvain Pineau (sylvain-pineau)]

Sorry for the delay (and thanks for the email detailing all the changes).

I'd keep nvme/mdadm original values. We've found IoT devices with such setup and surprisingly low speed.

[Rod Smith (rodsmith)]

OK, I've reverted the changes to nvme and mdadm, but kept everything else. Based on earlier reviews, I'll go ahead and merge this variant.

review: Approve

diff --git a/bin/disk_read_performance_test b/bin/disk_read_performance_test
index 8d6d6a0..bcf1c75 100755
--- a/bin/disk_read_performance_test
+++ b/bin/disk_read_performance_test
@@ -13,6 +13,10 @@ for disk in $@; do
 
   disk_type=`udevadm info --name /dev/$disk --query property | grep "ID_BUS" | awk '{gsub(/ID_BUS=/," ")}{printf $1}'`
   dev_path=`udevadm info --name /dev/$disk --query property | grep "DEVPATH" | awk '{gsub(/DEVPATH=/," ")}{printf $1}'`
+  # /sys/block/$disk/queue/rotational was added with Linux 2.6.29. If file is
+  # not present, test below will fail & disk will be considered an HDD, not
+  # an SSD.
+  rotational=`cat /sys/block/$disk/queue/rotational`
   if [[ $dev_path =~ dm ]]; then
     disk_type="devmapper"
   fi
@@ -25,6 +29,12 @@ for disk in $@; do
   if [[ $dev_path =~ mmc ]]; then
     disk_type="mmc"
   fi
+  if [[ $dev_path =~ pmem ]]; then
+    disk_type="nvdimm"
+  fi
+  if [[ ($disk_type == "scsi" || $disk_type == "ata") && $rotational == 0 ]]; then
+    disk_type="ssd"
+  fi
   if [ -z "$disk_type" ]; then
   echo "ERROR: disk type not recognized"
     exit 1
@@ -49,7 +59,11 @@ for disk in $@; do
     "ide" ) MIN_BUF_READ=40;;
     "mmc" ) MIN_BUF_READ=$DEFAULT_BUF_READ;;
     "nvme" ) MIN_BUF_READ=200;;
+    "nvdimm" ) MIN_BUF_READ=500;;
     "mdadm" ) MIN_BUF_READ=80;;
+    "ata" ) MIN_BUF_READ=100;;
+    "scsi" ) MIN_BUF_READ=100;;
+    "ssd" ) MIN_BUF_READ=200;;
     *     ) MIN_BUF_READ=$DEFAULT_BUF_READ;;
   esac
   echo "INFO: $disk_type: Using $MIN_BUF_READ MB/sec as the minimum throughput speed"