libstorage / libstoragemgmt

A library for storage management
https://libstorage.github.io/libstoragemgmt-doc/
GNU Lesser General Public License v2.1
82 stars 32 forks source link

Use one-byte length for VPD inquiry commands #444

Closed dmick closed 3 years ago

dmick commented 3 years ago

Idea from sg3_utils; it's turned out that certain controllers (Areca ARC-1680 series) cause timeouts and bus resets with longer requested lengths in these commands.

Signed-off-by: Dan Mick dmick@redhat.com

CI Kick

dmick commented 3 years ago

I'd like to test this against the failing hardware, so I'm starting to write a lowlevel API exerciser

dmick commented 3 years ago

This code reliably caused bus resets and errors without the fix and runs correctly with it. It does return 0, which may not be correct, but it runs without errors. the PIV bit is 0 in the returned hex (using sg_inq, as well):

# sg_inq -H --page 0x83 /dev/sde VPD INQUIRY, page code=0x83: 00 00 83 00 0c 01 02 00 08 00 1b 4d 20 00 00 00 00 ..........M ....

#include <stdio.h>
#include <libstoragemgmt/libstoragemgmt.h>

void
main()
{
        lsm_disk_link_type lt;
        lsm_error *lep;

        lsm_local_disk_link_type_get("/dev/sde", &lt, &lep);
        printf("link_type: %d\n", lt);
}
dmick commented 3 years ago

With these changes to tester.c:

--- a/test/tester.c
+++ b/test/tester.c
@@ -30,6 +30,7 @@
 #include <sys/stat.h>
 #include <time.h>
 #include <unistd.h>
+#pragma GCC diagnostic ignored "-Wunused-function"

 static int compare_battery(lsm_battery *l, lsm_battery *r);

@@ -4101,8 +4102,9 @@ Suite *lsm_suite(void) {
     Suite *s = suite_create("libStorageMgmt");

     TCase *basic = tcase_create("Basic");
-    tcase_add_checked_fixture(basic, setup, teardown);

+#if 0
+    tcase_add_checked_fixture(basic, setup, teardown);
     tcase_add_test(basic, test_volume_vpd_check);
     tcase_add_test(basic, test_initiator_id_verification);
     tcase_add_test(basic, test_target_ports);
@@ -4142,9 +4144,11 @@ Suite *lsm_suite(void) {
     tcase_add_test(basic, test_volume_raid_create);
     tcase_add_test(basic, test_volume_ident_led_on);
     tcase_add_test(basic, test_volume_ident_led_off);
+#endif
     tcase_add_test(basic, test_local_disk_vpd83_search);
     tcase_add_test(basic, test_local_disk_serial_num_get);
     tcase_add_test(basic, test_local_disk_vpd83_get);
+#if 0
     tcase_add_test(basic, test_read_cache_pct_update);
     tcase_add_test(basic, test_local_disk_list);
     tcase_add_test(basic, test_local_disk_rpm_get);
@@ -4155,6 +4159,7 @@ Suite *lsm_suite(void) {
     tcase_add_test(basic, test_volume_pdc_update);
     tcase_add_test(basic, test_volume_wcp_update);
     tcase_add_test(basic, test_volume_rcp_update);
+#endif
     tcase_add_test(basic, test_local_disk_ident_led);
     tcase_add_test(basic, test_local_disk_fault_led);
     tcase_add_test(basic, test_local_disk_led_status_get);

I get this output:

# LSM_TEST_RUNDIR=$(pwd)/test_rundir CK_VERBOSITY=verbose test/tester
Running suite(s): libStorageMgmt
lsm_local_disk_ident_led_on(): not supported disk /dev/sda
lsm_local_disk_ident_led_off(): not supported disk /dev/sda
lsm_local_disk_ident_led_on(): not supported disk /dev/sdb
lsm_local_disk_ident_led_off(): not supported disk /dev/sdb
lsm_local_disk_ident_led_on(): not supported disk /dev/sdc
lsm_local_disk_ident_led_off(): not supported disk /dev/sdc
lsm_local_disk_ident_led_on(): not supported disk /dev/sdd
lsm_local_disk_ident_led_off(): not supported disk /dev/sdd
lsm_local_disk_fault_led_on(): not supported disk /dev/sda
lsm_local_disk_fault_led_off(): not supported disk /dev/sda
lsm_local_disk_fault_led_on(): not supported disk /dev/sdb
lsm_local_disk_fault_led_off(): not supported disk /dev/sdb
lsm_local_disk_fault_led_on(): not supported disk /dev/sdc
lsm_local_disk_fault_led_off(): not supported disk /dev/sdc
lsm_local_disk_fault_led_on(): not supported disk /dev/sdd
lsm_local_disk_fault_led_off(): not supported disk /dev/sdd
100%: Checks: 7, Failures: 0, Errors: 0

There are 11 tests still uncommented. I'm not yet sure what that means.