Commit 7ea4a778 authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Tomasz Zawadzki
Browse files

bdev/nvme: Consolidate populate_namespace_cb() calls for error cases into connect_attach_cb() 



Consolidate populate_namespaces_cb() calls for error cases into
connect_attach_cb().  Then remove ctx parameter from
bdev_nvme_add_secondary_trid() because it is not necessary now.

The next patch will inline _nvme_bdev_ctrlr_create() into
nvme_bdev_ctrlr_create().

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


Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
parent d8e18534

module/bdev/nvme/bdev_nvme.c

+14 −17
Original line number Diff line number Diff line
@@ -1927,7 +1927,7 @@ err_init_mutex: 
	return rc;

}



static void

static int

nvme_bdev_ctrlr_create(struct spdk_nvme_ctrlr *ctrlr,

		       const char *name,

		       const struct spdk_nvme_transport_id *trid,
@@ -1940,16 +1940,11 @@ nvme_bdev_ctrlr_create(struct spdk_nvme_ctrlr *ctrlr, 
	rc = _nvme_bdev_ctrlr_create(ctrlr, name, trid, prchk_flags, &nvme_bdev_ctrlr);

	if (rc != 0) {

		SPDK_ERRLOG("Failed to create new NVMe controller\n");

		goto err;

		return rc;

	}



	nvme_ctrlr_populate_namespaces(nvme_bdev_ctrlr, ctx);

	return;



err:

	if (ctx != NULL) {

		populate_namespaces_cb(ctx, 0, rc);

	}

	return 0;

}



static void
@@ -2275,11 +2270,10 @@ _bdev_nvme_add_secondary_trid(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr, 
 * nvme_bdev_ctrlr for failover. After checking if it can access the same

 * namespaces as the primary path, it is disconnected until failover occurs.

 */

static void

static int

bdev_nvme_add_secondary_trid(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr,

			     struct spdk_nvme_ctrlr *new_ctrlr,

			     struct spdk_nvme_transport_id *trid,

			     struct nvme_async_probe_ctx *ctx)

			     struct spdk_nvme_transport_id *trid)

{

	int rc;
@@ -2304,9 +2298,7 @@ exit: 


	spdk_nvme_detach(new_ctrlr);



	if (ctx != NULL) {

		populate_namespaces_cb(ctx, 0, rc);

	}

	return rc;

}



static void
@@ -2316,17 +2308,22 @@ connect_attach_cb(void *cb_ctx, const struct spdk_nvme_transport_id *trid, 
	struct spdk_nvme_ctrlr_opts *user_opts = cb_ctx;

	struct nvme_bdev_ctrlr	*nvme_bdev_ctrlr;

	struct nvme_async_probe_ctx *ctx;

	int rc;



	ctx = SPDK_CONTAINEROF(user_opts, struct nvme_async_probe_ctx, opts);

	ctx->ctrlr_attached = true;



	nvme_bdev_ctrlr = nvme_bdev_ctrlr_get_by_name(ctx->base_name);

	if (nvme_bdev_ctrlr) {

		bdev_nvme_add_secondary_trid(nvme_bdev_ctrlr, ctrlr, &ctx->trid, ctx);

		rc = bdev_nvme_add_secondary_trid(nvme_bdev_ctrlr, ctrlr, &ctx->trid);

	} else {

		rc = nvme_bdev_ctrlr_create(ctrlr, ctx->base_name, &ctx->trid, ctx->prchk_flags, ctx);

		if (rc == 0) {

			return;

		}

	}



	nvme_bdev_ctrlr_create(ctrlr, ctx->base_name, &ctx->trid, ctx->prchk_flags, ctx);

	populate_namespaces_cb(ctx, 0, rc);

}



static int

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

+24 −12
Original line number Diff line number Diff line
@@ -1005,7 +1005,8 @@ test_create_ctrlr(void) 


	ut_init_trid(&trid);



	nvme_bdev_ctrlr_create(&ctrlr, "nvme0", &trid, 0, NULL);

	rc = nvme_bdev_ctrlr_create(&ctrlr, "nvme0", &trid, 0, NULL);

	CU_ASSERT(rc == 0);



	CU_ASSERT(nvme_bdev_ctrlr_get_by_name("nvme0") != NULL);
@@ -1035,7 +1036,8 @@ test_reset_ctrlr(void) 


	set_thread(0);



	nvme_bdev_ctrlr_create(&ctrlr, "nvme0", &trid, 0, NULL);

	rc = nvme_bdev_ctrlr_create(&ctrlr, "nvme0", &trid, 0, NULL);

	CU_ASSERT(rc == 0);



	nvme_bdev_ctrlr = nvme_bdev_ctrlr_get_by_name("nvme0");

	SPDK_CU_ASSERT_FATAL(nvme_bdev_ctrlr != NULL);
@@ -1140,7 +1142,8 @@ test_race_between_reset_and_destruct_ctrlr(void) 


	set_thread(0);



	nvme_bdev_ctrlr_create(&ctrlr, "nvme0", &trid, 0, NULL);

	rc = nvme_bdev_ctrlr_create(&ctrlr, "nvme0", &trid, 0, NULL);

	CU_ASSERT(rc == 0);



	nvme_bdev_ctrlr = nvme_bdev_ctrlr_get_by_name("nvme0");

	SPDK_CU_ASSERT_FATAL(nvme_bdev_ctrlr != NULL);
@@ -1216,7 +1219,8 @@ test_failover_ctrlr(void) 


	set_thread(0);



	nvme_bdev_ctrlr_create(&ctrlr, "nvme0", &trid1, 0, NULL);

	rc = nvme_bdev_ctrlr_create(&ctrlr, "nvme0", &trid1, 0, NULL);

	CU_ASSERT(rc == 0);



	nvme_bdev_ctrlr = nvme_bdev_ctrlr_get_by_name("nvme0");

	SPDK_CU_ASSERT_FATAL(nvme_bdev_ctrlr != NULL);
@@ -1278,7 +1282,8 @@ test_failover_ctrlr(void) 
	set_thread(0);



	/* Second, test two trids case. */

	bdev_nvme_add_secondary_trid(nvme_bdev_ctrlr, &ctrlr, &trid2, NULL);

	rc = bdev_nvme_add_secondary_trid(nvme_bdev_ctrlr, &ctrlr, &trid2);

	CU_ASSERT(rc == 0);



	curr_trid = TAILQ_FIRST(&nvme_bdev_ctrlr->trids);

	SPDK_CU_ASSERT_FATAL(curr_trid != NULL);
@@ -1610,7 +1615,8 @@ test_reconnect_qpair(void) 
	ut_init_trid(&trid);

	TAILQ_INIT(&ctrlr.active_io_qpairs);



	nvme_bdev_ctrlr_create(&ctrlr, "nvme0", &trid, 0, NULL);

	rc = nvme_bdev_ctrlr_create(&ctrlr, "nvme0", &trid, 0, NULL);

	CU_ASSERT(rc == 0);



	nvme_bdev_ctrlr = nvme_bdev_ctrlr_get_by_name("nvme0");

	SPDK_CU_ASSERT_FATAL(nvme_bdev_ctrlr != NULL);
@@ -1930,12 +1936,14 @@ test_remove_trid(void) 


	set_thread(0);



	nvme_bdev_ctrlr_create(&ctrlr, "nvme0", &trid1, 0, NULL);

	rc = nvme_bdev_ctrlr_create(&ctrlr, "nvme0", &trid1, 0, NULL);

	CU_ASSERT(rc == 0);



	nvme_bdev_ctrlr = nvme_bdev_ctrlr_get_by_name("nvme0");

	SPDK_CU_ASSERT_FATAL(nvme_bdev_ctrlr != NULL);



	bdev_nvme_add_secondary_trid(nvme_bdev_ctrlr, &ctrlr, &trid2, NULL);

	rc = bdev_nvme_add_secondary_trid(nvme_bdev_ctrlr, &ctrlr, &trid2);

	CU_ASSERT(rc == 0);



	/* trid3 is not in the registered list. */

	rc = bdev_nvme_delete("nvme0", &trid3);
@@ -1949,7 +1957,8 @@ test_remove_trid(void) 
		CU_ASSERT(spdk_nvme_transport_id_compare(&ctrid->trid, &trid2) != 0);

	}



	bdev_nvme_add_secondary_trid(nvme_bdev_ctrlr, &ctrlr, &trid3, NULL);

	rc = bdev_nvme_add_secondary_trid(nvme_bdev_ctrlr, &ctrlr, &trid3);

	CU_ASSERT(rc == 0);



	/* trid1 is currently used and trid3 is an alternative path.

	 * If we remove trid1, path is changed to trid3.
@@ -1978,12 +1987,14 @@ test_remove_trid(void) 


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



	nvme_bdev_ctrlr_create(&ctrlr, "nvme0", &trid1, 0, NULL);

	rc = nvme_bdev_ctrlr_create(&ctrlr, "nvme0", &trid1, 0, NULL);

	CU_ASSERT(rc == 0);



	nvme_bdev_ctrlr = nvme_bdev_ctrlr_get_by_name("nvme0");

	SPDK_CU_ASSERT_FATAL(nvme_bdev_ctrlr != NULL);



	bdev_nvme_add_secondary_trid(nvme_bdev_ctrlr, &ctrlr, &trid2, NULL);

	rc = bdev_nvme_add_secondary_trid(nvme_bdev_ctrlr, &ctrlr, &trid2);

	CU_ASSERT(rc == 0);



	/* If trid is not specified, nvme_bdev_ctrlr itself is removed. */

	rc = bdev_nvme_delete("nvme0", NULL);
@@ -2194,7 +2205,8 @@ test_get_io_qpair(void) 


	set_thread(0);



	nvme_bdev_ctrlr_create(&ctrlr, "nvme0", &trid, 0, NULL);

	rc = nvme_bdev_ctrlr_create(&ctrlr, "nvme0", &trid, 0, NULL);

	CU_ASSERT(rc == 0);



	nvme_bdev_ctrlr = nvme_bdev_ctrlr_get_by_name("nvme0");

	SPDK_CU_ASSERT_FATAL(nvme_bdev_ctrlr != NULL);