Commit 613d4413 authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Tomasz Zawadzki
Browse files

bdev/nvme: Process only the head of linked list, nvme_ns->bdevs 



By the last changes, not only standard namespace but also ocssd
namespace has only one nvme_bdev, and standard namespace processes
only the head of nvme_ns->bdevs.

This patch changes the common and standard namespace specific
part to process only the head of nvme_ns->bdevs.

The following patch will replace the linked list nvme_ns->bdevs
by the pointer nvme_ns->bdev.

Add a particular error case that nvme_bdev is failed to create even
if ctrlr has one namespace. If ctrlr has one namespace but the
corresponding bdev is failed to create, nvme_ns->populated should
be false and hence nvme_ns->bdevs should not be accessed. However
the code had not assumed such case.

Signed-off-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I5495882fad8c8a012305177179a46d4373ba75f5
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/5800


Community-CI: Broadcom CI
Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
parent 9772b580

module/bdev/nvme/bdev_nvme.c

+18 −13
Original line number Diff line number Diff line
@@ -1345,9 +1345,10 @@ nvme_ctrlr_depopulate_namespace_done(struct nvme_bdev_ns *nvme_ns) 
static void

nvme_ctrlr_depopulate_standard_namespace(struct nvme_bdev_ns *nvme_ns)

{

	struct nvme_bdev *bdev, *tmp;

	struct nvme_bdev *bdev;



	TAILQ_FOREACH_SAFE(bdev, &nvme_ns->bdevs, tailq, tmp) {

	bdev = TAILQ_FIRST(&nvme_ns->bdevs);

	if (bdev != NULL) {

		spdk_bdev_unregister(&bdev->disk, NULL, NULL);

	}
@@ -1421,6 +1422,7 @@ nvme_ctrlr_populate_namespaces(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr, 
			ns = spdk_nvme_ctrlr_get_ns(ctrlr, nsid);

			num_sectors = spdk_nvme_ns_get_num_sectors(ns);

			bdev = TAILQ_FIRST(&nvme_ns->bdevs);

			assert(bdev != NULL);

			if (bdev->disk.blockcnt != num_sectors) {

				SPDK_NOTICELOG("NSID %u is resized: bdev name %s, old size %" PRIu64 ", new size %" PRIu64 "\n",

					       nsid,
@@ -1812,7 +1814,7 @@ nvme_ctrlr_populate_namespaces_done(struct nvme_async_probe_ctx *ctx) 
{

	struct nvme_bdev_ctrlr	*nvme_bdev_ctrlr;

	struct nvme_bdev_ns	*nvme_ns;

	struct nvme_bdev	*nvme_bdev, *tmp;

	struct nvme_bdev	*nvme_bdev;

	uint32_t		i, nsid;

	size_t			j;
@@ -1831,7 +1833,11 @@ nvme_ctrlr_populate_namespaces_done(struct nvme_async_probe_ctx *ctx) 
			continue;

		}

		assert(nvme_ns->id == nsid);

		TAILQ_FOREACH_SAFE(nvme_bdev, &nvme_ns->bdevs, tailq, tmp) {

		nvme_bdev = TAILQ_FIRST(&nvme_ns->bdevs);

		if (nvme_bdev == NULL) {

			assert(nvme_ns->type == NVME_BDEV_NS_OCSSD);

			continue;

		}

		if (j < ctx->count) {

			ctx->names[j] = nvme_bdev->disk.name;

			j++;
@@ -1842,7 +1848,6 @@ nvme_ctrlr_populate_namespaces_done(struct nvme_async_probe_ctx *ctx) 
			return;

		}

	}

	}



	populate_namespaces_cb(ctx, j, 0);

}

test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c

+37 −1
Original line number Diff line number Diff line
@@ -209,6 +209,7 @@ static TAILQ_HEAD(, spdk_nvme_ctrlr) g_ut_attached_ctrlrs = TAILQ_HEAD_INITIALIZ 
			g_ut_attached_ctrlrs);

static int g_ut_attach_ctrlr_status;

static size_t g_ut_attach_bdev_count;

static int g_ut_register_bdev_status;



static void

ut_init_trid(struct spdk_nvme_transport_id *trid)
@@ -744,7 +745,7 @@ spdk_nvme_poll_group_remove(struct spdk_nvme_poll_group *group, 
int

spdk_bdev_register(struct spdk_bdev *bdev)

{

	return 0;

	return g_ut_register_bdev_status;

}



void
@@ -1332,6 +1333,41 @@ test_attach_ctrlr(void) 
	CU_ASSERT(nvme_bdev_ctrlr_get_by_name("nvme0") == NULL);



	ut_detach_ctrlr(ctrlr);



	/* Ctrlr has one namespace but one nvme_bdev_ctrlr with no namespace is

	 * created because creating one nvme_bdev failed.

	 */

	ctrlr = ut_attach_ctrlr(&trid, 1);

	SPDK_CU_ASSERT_FATAL(ctrlr != NULL);



	ctrlr->ns[0].is_active = true;

	g_ut_register_bdev_status = -EINVAL;

	g_ut_attach_bdev_count = 0;



	rc = bdev_nvme_create(&trid, &hostid, "nvme0", attached_names, 32, NULL, 0,

			      attach_ctrlr_done, NULL, NULL);

	CU_ASSERT(rc == 0);



	spdk_delay_us(1000);

	poll_threads();



	nvme_bdev_ctrlr = nvme_bdev_ctrlr_get_by_name("nvme0");

	SPDK_CU_ASSERT_FATAL(nvme_bdev_ctrlr != NULL);

	CU_ASSERT(nvme_bdev_ctrlr->ctrlr == ctrlr);

	CU_ASSERT(nvme_bdev_ctrlr->num_ns == 1);



	CU_ASSERT(attached_names[0] == NULL);



	rc = bdev_nvme_delete("nvme0");

	CU_ASSERT(rc == 0);



	poll_threads();



	CU_ASSERT(nvme_bdev_ctrlr_get_by_name("nvme0") == NULL);



	ut_detach_ctrlr(ctrlr);



	g_ut_register_bdev_status = 0;

}



static void