Commit 83f27434 authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Tomasz Zawadzki
Browse files

bdev/nvme: Merge bdev_nvme_remove_trid() into bdev_nvme_delete() 



This will make us easier to maintain the operation to delete
nvme_bdev_ctrlr and its trids. The added unit test cases guard us
from degradation.

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


Community-CI: Broadcom CI
Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
parent 3eb0b6b1

module/bdev/nvme/bdev_nvme.c

+11 −29
Original line number Diff line number Diff line
@@ -2122,7 +2122,7 @@ bdev_nvme_create(struct spdk_nvme_transport_id *trid, 
}



int

bdev_nvme_remove_trid(const char *name, struct spdk_nvme_transport_id *trid)

bdev_nvme_delete(const char *name, const struct spdk_nvme_transport_id *trid)

{

	struct nvme_bdev_ctrlr		*nvme_bdev_ctrlr;

	struct nvme_bdev_ctrlr_trid	*ctrlr_trid, *tmp_trid;
@@ -2137,19 +2137,24 @@ bdev_nvme_remove_trid(const char *name, struct spdk_nvme_transport_id *trid) 
		return -ENODEV;

	}



	/* case 1: we are currently using the path to be removed. */

	/* case 1: remove the controller itself. */

	if (trid == NULL) {

		return _bdev_nvme_delete(nvme_bdev_ctrlr, false);

	}



	/* case 2: we are currently using the path to be removed. */

	if (!spdk_nvme_transport_id_compare(trid, nvme_bdev_ctrlr->connected_trid)) {

		ctrlr_trid = TAILQ_FIRST(&nvme_bdev_ctrlr->trids);

		assert(nvme_bdev_ctrlr->connected_trid == &ctrlr_trid->trid);

		/* case 1A: the current path is the only path. */

		/* case 2A: the current path is the only path. */

		if (!TAILQ_NEXT(ctrlr_trid, link)) {

			return bdev_nvme_delete(name);

			return _bdev_nvme_delete(nvme_bdev_ctrlr, false);

		}



		/* case 1B: there is an alternative path. */

		return bdev_nvme_failover(nvme_bdev_ctrlr, true);

	}

	/* case 2: We are not using the specified path. */

	/* case 3: We are not using the specified path. */

	TAILQ_FOREACH_SAFE(ctrlr_trid, &nvme_bdev_ctrlr->trids, link, tmp_trid) {

		if (!spdk_nvme_transport_id_compare(&ctrlr_trid->trid, trid)) {

			TAILQ_REMOVE(&nvme_bdev_ctrlr->trids, ctrlr_trid, link);
@@ -2158,33 +2163,10 @@ bdev_nvme_remove_trid(const char *name, struct spdk_nvme_transport_id *trid) 
		}

	}



	/* case 2A: The address isn't even in the registered list. */

	/* case 3A: The address isn't even in the registered list. */

	return -ENXIO;

}



int

bdev_nvme_delete(const char *name)

{

	struct nvme_bdev_ctrlr *nvme_bdev_ctrlr;



	if (name == NULL) {

		return -EINVAL;

	}



	pthread_mutex_lock(&g_bdev_nvme_mutex);



	nvme_bdev_ctrlr = nvme_bdev_ctrlr_get_by_name(name);

	if (nvme_bdev_ctrlr == NULL) {

		pthread_mutex_unlock(&g_bdev_nvme_mutex);

		SPDK_ERRLOG("Failed to find NVMe controller\n");

		return -ENODEV;

	}



	pthread_mutex_unlock(&g_bdev_nvme_mutex);



	return _bdev_nvme_delete(nvme_bdev_ctrlr, false);

}



static int

bdev_nvme_library_init(void)

{

module/bdev/nvme/bdev_nvme.h

+4 −3
Original line number Diff line number Diff line
@@ -82,12 +82,13 @@ int bdev_nvme_create(struct spdk_nvme_transport_id *trid, 
struct spdk_nvme_ctrlr *bdev_nvme_get_ctrlr(struct spdk_bdev *bdev);



/**

 * Delete NVMe controller with all bdevs on top of it.

 * Requires to pass name of NVMe controller.

 * Delete NVMe controller with all bdevs on top of it, or delete the specified path

 * if there is any alternative path. Requires to pass name of NVMe controller.

 *

 * \param name NVMe controller name

 * \param trid The specified transport ID to remove (optional)

 * \return zero on success, -EINVAL on wrong parameters or -ENODEV if controller is not found

 */

int bdev_nvme_delete(const char *name);

int bdev_nvme_delete(const char *name, const struct spdk_nvme_transport_id *trid);



#endif /* SPDK_BDEV_NVME_H */

module/bdev/nvme/bdev_nvme_rpc.c

+2 −2
Original line number Diff line number Diff line
@@ -589,9 +589,9 @@ rpc_bdev_nvme_detach_controller(struct spdk_jsonrpc_request *request, 
			goto cleanup;

		}

		memcpy(trid.subnqn, req.subnqn, len + 1);

		rc = bdev_nvme_remove_trid(req.name, &trid);

		rc = bdev_nvme_delete(req.name, &trid);

	} else {

		rc = bdev_nvme_delete(req.name);

		rc = bdev_nvme_delete(req.name, NULL);

	}



	if (rc != 0) {

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

+30 −15
Original line number Diff line number Diff line
@@ -834,7 +834,7 @@ test_create_ctrlr(void) 


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



	rc = bdev_nvme_delete("nvme0");

	rc = bdev_nvme_delete("nvme0", NULL);

	CU_ASSERT(rc == 0);



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


	poll_threads();



	rc = bdev_nvme_delete("nvme0");

	rc = bdev_nvme_delete("nvme0", NULL);

	CU_ASSERT(rc == 0);



	poll_threads();
@@ -986,7 +986,7 @@ test_race_between_reset_and_destruct_ctrlr(void) 
	/* Try destructing ctrlr while ctrlr is being reset, but it will be deferred. */

	set_thread(0);



	rc = bdev_nvme_delete("nvme0");

	rc = bdev_nvme_delete("nvme0", NULL);

	CU_ASSERT(rc == 0);

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

	CU_ASSERT(nvme_bdev_ctrlr->destruct == true);
@@ -1148,7 +1148,7 @@ test_failover_ctrlr(void) 


	poll_threads();



	rc = bdev_nvme_delete("nvme0");

	rc = bdev_nvme_delete("nvme0", NULL);

	CU_ASSERT(rc == 0);



	poll_threads();
@@ -1230,7 +1230,7 @@ test_pending_reset(void) 
	set_thread(0);





	rc = bdev_nvme_delete("nvme0");

	rc = bdev_nvme_delete("nvme0", NULL);

	CU_ASSERT(rc == 0);



	poll_threads();
@@ -1299,7 +1299,7 @@ test_attach_ctrlr(void) 
	CU_ASSERT(nvme_bdev_ctrlr->ctrlr == ctrlr);

	CU_ASSERT(nvme_bdev_ctrlr->num_ns == 0);



	rc = bdev_nvme_delete("nvme0");

	rc = bdev_nvme_delete("nvme0", NULL);

	CU_ASSERT(rc == 0);



	poll_threads();
@@ -1331,7 +1331,7 @@ test_attach_ctrlr(void) 
	CU_ASSERT(attached_names[0] != NULL && strcmp(attached_names[0], "nvme0n1") == 0);

	attached_names[0] = NULL;



	rc = bdev_nvme_delete("nvme0");

	rc = bdev_nvme_delete("nvme0", NULL);

	CU_ASSERT(rc == 0);



	poll_threads();
@@ -1364,7 +1364,7 @@ test_attach_ctrlr(void) 


	CU_ASSERT(attached_names[0] == NULL);



	rc = bdev_nvme_delete("nvme0");

	rc = bdev_nvme_delete("nvme0", NULL);

	CU_ASSERT(rc == 0);



	poll_threads();
@@ -1423,7 +1423,7 @@ test_reconnect_qpair(void) 


	poll_threads();



	rc = bdev_nvme_delete("nvme0");

	rc = bdev_nvme_delete("nvme0", NULL);

	CU_ASSERT(rc == 0);



	poll_threads();
@@ -1502,7 +1502,7 @@ test_aer_cb(void) 
	CU_ASSERT(nvme_bdev_ctrlr->namespaces[3]->populated == true);

	CU_ASSERT(bdev->disk.blockcnt == 2048);



	rc = bdev_nvme_delete("nvme0");

	rc = bdev_nvme_delete("nvme0", NULL);

	CU_ASSERT(rc == 0);



	poll_threads();
@@ -1644,7 +1644,7 @@ test_submit_nvme_cmd(void) 


	poll_threads();



	rc = bdev_nvme_delete("nvme0");

	rc = bdev_nvme_delete("nvme0", NULL);

	CU_ASSERT(rc == 0);



	poll_threads();
@@ -1677,11 +1677,11 @@ test_remove_trid(void) 
	CU_ASSERT(rc == 0);



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

	rc = bdev_nvme_remove_trid("nvme0", &trid3);

	rc = bdev_nvme_delete("nvme0", &trid3);

	CU_ASSERT(rc == -ENXIO);



	/* trid2 is not used, and simply removed. */

	rc = bdev_nvme_remove_trid("nvme0", &trid2);

	rc = bdev_nvme_delete("nvme0", &trid2);

	CU_ASSERT(rc == 0);

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

	TAILQ_FOREACH(ctrid, &nvme_bdev_ctrlr->trids, link) {
@@ -1694,7 +1694,7 @@ test_remove_trid(void) 
	/* trid1 is currently used and trid3 is an alternative path.

	 * If we remove trid1, path is changed to trid3.

	 */

	rc = bdev_nvme_remove_trid("nvme0", &trid1);

	rc = bdev_nvme_delete("nvme0", &trid1);

	CU_ASSERT(rc == 0);

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

	CU_ASSERT(nvme_bdev_ctrlr->resetting == true);
@@ -1710,7 +1710,22 @@ test_remove_trid(void) 
	/* trid3 is the current and only path. If we remove trid3, the corresponding

	 * nvme_bdev_ctrlr is removed.

	 */

	rc = bdev_nvme_remove_trid("nvme0", &trid3);

	rc = bdev_nvme_delete("nvme0", &trid3);

	CU_ASSERT(rc == 0);

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



	poll_threads();



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



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

	CU_ASSERT(rc == 0);



	rc = bdev_nvme_add_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);

	CU_ASSERT(rc == 0);

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