scsi: handle return status of spdk_scsi_lun_claim(lun)

This is necessary to prevent claiming the same LUN twice
and properly cleanup in case of an error during spdk_scsi_dev_construct.

This patch addresses three issues:
- spdk_scsi_lun_claim error is correctly handled in spdk_scsi_dev_add_lun
- on error when constructing scsi dev, it is now correctly removed along with attached luns
- spdk_scsi_dev_destruct not only unclaims, but calls spdk_scsi_lun_destruct on each lun in dev

Unit tests relevant to this behaviour are changed to show this functionality is now working.

Signed-off-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Change-Id: I111c320f875e5003e3f1f7748a2630097301ce1b
This commit is contained in:
Tomasz Zawadzki 2017-02-07 14:08:06 +01:00
parent 86278ab90e
commit 90b0873665
3 changed files with 29 additions and 12 deletions

View File

@ -82,23 +82,32 @@ spdk_scsi_dev_destruct(struct spdk_scsi_dev *dev)
} }
spdk_scsi_lun_unclaim(dev->lun[i]); spdk_scsi_lun_unclaim(dev->lun[i]);
spdk_scsi_lun_destruct(dev->lun[i]);
dev->lun[i] = NULL; dev->lun[i] = NULL;
} }
free_dev(dev); free_dev(dev);
} }
static void static int
spdk_scsi_dev_add_lun(struct spdk_scsi_dev *dev, spdk_scsi_dev_add_lun(struct spdk_scsi_dev *dev,
struct spdk_scsi_lun *lun, int id) struct spdk_scsi_lun *lun, int id)
{ {
spdk_scsi_lun_claim(lun); int rc;
rc = spdk_scsi_lun_claim(lun);
if (rc < 0) {
return rc;
}
lun->id = id; lun->id = id;
lun->dev = dev; lun->dev = dev;
dev->lun[id] = lun; dev->lun[id] = lun;
if (dev->maxlun <= id) { if (dev->maxlun <= id) {
dev->maxlun = id + 1; dev->maxlun = id + 1;
} }
return 0;
} }
void void
@ -133,8 +142,8 @@ spdk_scsi_dev_construct(const char *name, char *lun_name_list[], int *lun_id_lis
{ {
struct spdk_scsi_dev *dev; struct spdk_scsi_dev *dev;
struct spdk_bdev *bdev; struct spdk_bdev *bdev;
struct spdk_scsi_lun *lun; struct spdk_scsi_lun *lun = NULL;
int i; int i, rc;
if (num_luns == 0) { if (num_luns == 0) {
SPDK_ERRLOG("device %s: no LUNs specified\n", name); SPDK_ERRLOG("device %s: no LUNs specified\n", name);
@ -167,20 +176,27 @@ spdk_scsi_dev_construct(const char *name, char *lun_name_list[], int *lun_id_lis
for (i = 0; i < num_luns; i++) { for (i = 0; i < num_luns; i++) {
bdev = spdk_bdev_get_by_name(lun_name_list[i]); bdev = spdk_bdev_get_by_name(lun_name_list[i]);
if (bdev == NULL) { if (bdev == NULL) {
free_dev(dev); goto error;
return NULL;
} }
lun = spdk_scsi_lun_construct(bdev->name, bdev); lun = spdk_scsi_lun_construct(bdev->name, bdev);
if (lun == NULL) { if (lun == NULL) {
free_dev(dev); goto error;
return NULL;
} }
spdk_scsi_dev_add_lun(dev, lun, lun_id_list[i]); rc = spdk_scsi_dev_add_lun(dev, lun, lun_id_list[i]);
if (rc < 0) {
spdk_scsi_lun_destruct(lun);
goto error;
}
} }
return dev; return dev;
error:
spdk_scsi_dev_destruct(dev);
return NULL;
} }
void void

View File

@ -304,6 +304,7 @@ spdk_scsi_lun_construct(const char *name, struct spdk_bdev *bdev)
rc = spdk_scsi_lun_db_add(lun); rc = spdk_scsi_lun_db_add(lun);
if (rc < 0) { if (rc < 0) {
SPDK_ERRLOG("Unable to add LUN %s to DB\n", lun->name); SPDK_ERRLOG("Unable to add LUN %s to DB\n", lun->name);
spdk_bdev_unclaim(bdev);
free(lun); free(lun);
return NULL; return NULL;
} }

View File

@ -298,7 +298,7 @@ dev_construct_same_lun_two_devices(void)
dev2 = spdk_scsi_dev_construct("Name2", lun_name_list, lun_id_list, 1); dev2 = spdk_scsi_dev_construct("Name2", lun_name_list, lun_id_list, 1);
/* Fails to construct dev and returns NULL */ /* Fails to construct dev and returns NULL */
CU_ASSERT_TRUE(dev2 != NULL); /* dev2 should be NULL, this shows it is not currently working */ CU_ASSERT_TRUE(dev2 == NULL);
/* free the dev */ /* free the dev */
spdk_scsi_dev_destruct(dev); spdk_scsi_dev_destruct(dev);
@ -316,9 +316,9 @@ dev_construct_same_lun_one_device(void)
dev = spdk_scsi_dev_construct("Name", lun_name_list, lun_id_list, 2); dev = spdk_scsi_dev_construct("Name", lun_name_list, lun_id_list, 2);
/* Fails to construct dev and returns NULL */ /* Fails to construct dev and returns NULL */
CU_ASSERT_TRUE(dev != NULL); /* dev should be NULL, this shows it is not currently working */ CU_ASSERT_TRUE(dev == NULL);
CU_ASSERT(!TAILQ_EMPTY(&g_lun_head)); /* lun list should be empty, as none should be created */ CU_ASSERT(TAILQ_EMPTY(&g_lun_head));
} }
static void static void