From 36a8f75006a854044a1c341f8c80a64e5da28f07 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Thu, 5 Oct 2017 16:41:46 -0700 Subject: [PATCH] scsi: fix LUN 0 check in scsi_dev_construct When constructing a SCSI device, LUN 0 must be assigned to some LUN. However, the current code was assuming that LUN 0 had to be in the first array element of lun_id_list in spdk_scsi_dev_construct(). Combined with the scripts/rpc.py implementation that uses a Python (unordered) dictionary object to generate the lun_ids array, there could be cases where LUN 0 exists in the list but isn't first. Fix the check by allowing LUN 0 in any position of the lun_id_list array. Change-Id: I39f387ec238fcecca8d2d786d3d42c42a4790637 Signed-off-by: Daniel Verkamp Reviewed-on: https://review.gerrithub.io/381611 Reviewed-by: Reviewed-by: Jim Harris Tested-by: SPDK Automated Test System Reviewed-by: Karol Latecki --- lib/scsi/dev.c | 11 ++++++++- test/unit/lib/scsi/dev.c/dev_ut.c | 41 ++++++++++++++++++++++++++++--- 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/lib/scsi/dev.c b/lib/scsi/dev.c index 5e96d55d5..32748a337 100644 --- a/lib/scsi/dev.c +++ b/lib/scsi/dev.c @@ -132,6 +132,7 @@ spdk_scsi_dev_construct(const char *name, char *lun_name_list[], int *lun_id_lis struct spdk_scsi_dev *dev; struct spdk_bdev *bdev; struct spdk_scsi_lun *lun = NULL; + bool found_lun_0; int i, rc; if (num_luns == 0) { @@ -139,7 +140,15 @@ spdk_scsi_dev_construct(const char *name, char *lun_name_list[], int *lun_id_lis return NULL; } - if (lun_id_list[0] != 0) { + found_lun_0 = false; + for (i = 0; i < num_luns; i++) { + if (lun_id_list[i] == 0) { + found_lun_0 = true; + break; + } + } + + if (!found_lun_0) { SPDK_ERRLOG("device %s: no LUN 0 specified\n", name); return NULL; } diff --git a/test/unit/lib/scsi/dev.c/dev_ut.c b/test/unit/lib/scsi/dev.c/dev_ut.c index 357329b0b..9980659b4 100644 --- a/test/unit/lib/scsi/dev.c/dev_ut.c +++ b/test/unit/lib/scsi/dev.c/dev_ut.c @@ -36,6 +36,8 @@ #include "CUnit/Basic.h" #include "spdk_cunit.h" +#include "spdk/util.h" + #include "dev.c" #include "port.c" @@ -44,7 +46,10 @@ struct spdk_bdev { char name[100]; }; -static struct spdk_bdev g_bdev = {}; +static struct spdk_bdev g_bdevs[] = { + {"malloc0"}, + {"malloc1"}, +}; struct lun_entry { TAILQ_ENTRY(lun_entry) lun_entries; @@ -62,7 +67,7 @@ test_setup(void) const char * spdk_bdev_get_name(const struct spdk_bdev *bdev) { - return "test"; + return bdev->name; } static struct spdk_scsi_task * @@ -109,8 +114,15 @@ spdk_scsi_lun_destruct(struct spdk_scsi_lun *lun) struct spdk_bdev * spdk_bdev_get_by_name(const char *bdev_name) { - snprintf(g_bdev.name, sizeof(g_bdev.name), "%s", bdev_name); - return &g_bdev; + size_t i; + + for (i = 0; i < SPDK_COUNTOF(g_bdevs); i++) { + if (strcmp(bdev_name, g_bdevs[i].name) == 0) { + return &g_bdevs[i]; + } + } + + return NULL; } int @@ -292,6 +304,25 @@ dev_construct_success(void) CU_ASSERT(TAILQ_EMPTY(&g_lun_head)); } +static void +dev_construct_success_lun_zero_not_first(void) +{ + struct spdk_scsi_dev *dev; + char *lun_name_list[2] = {"malloc1", "malloc0"}; + int lun_id_list[2] = { 1, 0 }; + + dev = spdk_scsi_dev_construct("Name", lun_name_list, lun_id_list, 2, + SPDK_SPC_PROTOCOL_IDENTIFIER_ISCSI, NULL, NULL); + + /* Successfully constructs and returns a dev */ + CU_ASSERT_TRUE(dev != NULL); + + /* free the dev */ + spdk_scsi_dev_destruct(dev); + + CU_ASSERT(TAILQ_EMPTY(&g_lun_head)); +} + static void dev_construct_same_lun_two_devices(void) { @@ -574,6 +605,8 @@ main(int argc, char **argv) || CU_add_test(suite, "construct - null lun", dev_construct_null_lun) == NULL || CU_add_test(suite, "construct - success", dev_construct_success) == NULL + || CU_add_test(suite, "construct - success - LUN zero not first", + dev_construct_success_lun_zero_not_first) == NULL || CU_add_test(suite, "construct - same lun on two devices", dev_construct_same_lun_two_devices) == NULL || CU_add_test(suite, "construct - same lun on once device",