Commit 08e2210a authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Jim Harris
Browse files

bdev/nvme: Merge callback and spdk_nvme_detach() into bdev_nvme_add_trid() 



This change separates failover case more clearly, and will make
the following changes simpler.

Merge spdk_nvme_detach() and populate_namespaces_cb() into
bdev_nvme_add_secondary_trid(). Then change the return type of
bdev_nvme_add_secondary_trid() to void and move the comment to the
head of it.

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


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 avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarPaul Luse <paul.e.luse@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
parent 0e3de45d

module/bdev/nvme/bdev_nvme.c

+14 −13
Original line number Diff line number Diff line
@@ -2192,10 +2192,15 @@ _bdev_nvme_add_secondary_trid(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr, 
	return 0;

}



static int

/* This is the case that a secondary path is added to an existing

 * 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

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 spdk_nvme_transport_id *trid,

			     struct nvme_async_probe_ctx *ctx)

{

	int rc;
@@ -2217,7 +2222,12 @@ bdev_nvme_add_secondary_trid(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr, 


exit:

	pthread_mutex_unlock(&nvme_bdev_ctrlr->mutex);

	return rc;



	spdk_nvme_detach(new_ctrlr);



	if (ctx != NULL) {

		populate_namespaces_cb(ctx, 0, rc);

	}

}



static void
@@ -2227,22 +2237,13 @@ 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) {

		/* This is the case that a secondary path is added to an existing

		 * nvme_bdev_ctrlr for failover. After checking if it can access the same

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

		 */

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



		spdk_nvme_detach(ctrlr);



		populate_namespaces_cb(ctx, 0, rc);

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

		return;

	}

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

+4 −8
Original line number Diff line number Diff line
@@ -1235,8 +1235,7 @@ test_failover_ctrlr(void) 
	set_thread(0);



	/* Second, test two trids case. */

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

	CU_ASSERT(rc == 0);

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



	curr_trid = TAILQ_FIRST(&nvme_bdev_ctrlr->trids);

	SPDK_CU_ASSERT_FATAL(curr_trid != NULL);
@@ -1879,8 +1878,7 @@ test_remove_trid(void) 
	nvme_bdev_ctrlr = nvme_bdev_ctrlr_get_by_name("nvme0");

	SPDK_CU_ASSERT_FATAL(nvme_bdev_ctrlr != NULL);



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

	CU_ASSERT(rc == 0);

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



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

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

	}



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

	CU_ASSERT(rc == 0);

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



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

	 * If we remove trid1, path is changed to trid3.
@@ -1929,8 +1926,7 @@ test_remove_trid(void) 
	nvme_bdev_ctrlr = nvme_bdev_ctrlr_get_by_name("nvme0");

	SPDK_CU_ASSERT_FATAL(nvme_bdev_ctrlr != NULL);



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

	CU_ASSERT(rc == 0);

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



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

	rc = bdev_nvme_delete("nvme0", NULL);