From 90b0873665e3330e2a79132cc542eb60c443dac1 Mon Sep 17 00:00:00 2001
From: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Date: Tue, 7 Feb 2017 14:08:06 +0100
Subject: [PATCH] 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
---
 lib/scsi/dev.c             | 34 +++++++++++++++++++++++++---------
 lib/scsi/lun.c             |  1 +
 test/lib/scsi/dev/dev_ut.c |  6 +++---
 3 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/lib/scsi/dev.c b/lib/scsi/dev.c
index b36f001ca..15db10671 100644
--- a/lib/scsi/dev.c
+++ b/lib/scsi/dev.c
@@ -82,23 +82,32 @@ spdk_scsi_dev_destruct(struct spdk_scsi_dev *dev)
 		}
 
 		spdk_scsi_lun_unclaim(dev->lun[i]);
+		spdk_scsi_lun_destruct(dev->lun[i]);
 		dev->lun[i] = NULL;
 	}
 
 	free_dev(dev);
 }
 
-static void
+static int
 spdk_scsi_dev_add_lun(struct spdk_scsi_dev *dev,
 		      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->dev = dev;
 	dev->lun[id] = lun;
 	if (dev->maxlun <= id) {
 		dev->maxlun = id + 1;
 	}
+
+	return 0;
 }
 
 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_bdev *bdev;
-	struct spdk_scsi_lun *lun;
-	int i;
+	struct spdk_scsi_lun *lun = NULL;
+	int i, rc;
 
 	if (num_luns == 0) {
 		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++) {
 		bdev = spdk_bdev_get_by_name(lun_name_list[i]);
 		if (bdev == NULL) {
-			free_dev(dev);
-			return NULL;
+			goto error;
 		}
 
 		lun = spdk_scsi_lun_construct(bdev->name, bdev);
 		if (lun == NULL) {
-			free_dev(dev);
-			return NULL;
+			goto error;
 		}
 
-		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;
+
+error:
+	spdk_scsi_dev_destruct(dev);
+
+	return NULL;
 }
 
 void
diff --git a/lib/scsi/lun.c b/lib/scsi/lun.c
index 4270d9850..d382af448 100644
--- a/lib/scsi/lun.c
+++ b/lib/scsi/lun.c
@@ -304,6 +304,7 @@ spdk_scsi_lun_construct(const char *name, struct spdk_bdev *bdev)
 	rc = spdk_scsi_lun_db_add(lun);
 	if (rc < 0) {
 		SPDK_ERRLOG("Unable to add LUN %s to DB\n", lun->name);
+		spdk_bdev_unclaim(bdev);
 		free(lun);
 		return NULL;
 	}
diff --git a/test/lib/scsi/dev/dev_ut.c b/test/lib/scsi/dev/dev_ut.c
index 8415e5ff7..aa1c5d808 100644
--- a/test/lib/scsi/dev/dev_ut.c
+++ b/test/lib/scsi/dev/dev_ut.c
@@ -298,7 +298,7 @@ dev_construct_same_lun_two_devices(void)
 	dev2 = spdk_scsi_dev_construct("Name2", lun_name_list, lun_id_list, 1);
 
 	/* 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 */
 	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);
 
 	/* 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