Commit 88da8a91 authored by Ben Walker's avatar Ben Walker
Browse files

nvmf: spdk_nvmf_subsystem_remove_ns is no longer asynchronous 



Now that the resume path can correctly handle the case where a namespace
was removed and a new one added with the same nsid, this no longer needs
to be asynchronous.

Change-Id: I693045e66a7d4e75255b526d8f5ca5ef8695533e
Signed-off-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/459606


Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarSeth Howell <seth.howell5141@gmail.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent 316d5c7c

include/spdk/nvmf.h

+1 −4
Original line number Diff line number Diff line
@@ -588,13 +588,10 @@ uint32_t spdk_nvmf_subsystem_add_ns(struct spdk_nvmf_subsystem *subsystem, struc 
 *

 * \param subsystem Subsystem the namespace belong to.

 * \param nsid Namespace ID to be removed.

 * \param cb_fn Function to call when all thread local ns information has been updated

 * \param cb_arg Context for the above cb_fn

 *

 * \return 0 on success, -1 on failure.

 */

int spdk_nvmf_subsystem_remove_ns(struct spdk_nvmf_subsystem *subsystem, uint32_t nsid,

				  spdk_nvmf_subsystem_state_change_done cb_fn, void *cb_arg);

int spdk_nvmf_subsystem_remove_ns(struct spdk_nvmf_subsystem *subsystem, uint32_t nsid);



/**

 * Get the first allocated namespace in a subsystem.

lib/event/subsystems/nvmf/nvmf_rpc.c

+9 −23
Original line number Diff line number Diff line
@@ -1002,13 +1002,14 @@ nvmf_rpc_remove_ns_resumed(struct spdk_nvmf_subsystem *subsystem, 
}



static void

nvmf_rpc_remove_ns_remove_done(struct spdk_nvmf_subsystem *subsystem, void *cb_arg, int status)

nvmf_rpc_remove_ns_paused(struct spdk_nvmf_subsystem *subsystem,

			  void *cb_arg, int status)

{

	struct nvmf_rpc_remove_ns_ctx *ctx;



	ctx = cb_arg;

	struct nvmf_rpc_remove_ns_ctx *ctx = cb_arg;

	int ret;



	if (status != 0) {

	ret = spdk_nvmf_subsystem_remove_ns(subsystem, ctx->nsid);

	if (ret < 0) {

		SPDK_ERRLOG("Unable to remove namespace ID %u\n", ctx->nsid);

		spdk_jsonrpc_send_error_response(ctx->request, SPDK_JSONRPC_ERROR_INVALID_PARAMS,

						 "Invalid parameters");
@@ -1016,29 +1017,14 @@ nvmf_rpc_remove_ns_remove_done(struct spdk_nvmf_subsystem *subsystem, void *cb_a 
	}



	if (spdk_nvmf_subsystem_resume(subsystem, nvmf_rpc_remove_ns_resumed, ctx)) {

		if (!ctx->response_sent) {

			spdk_jsonrpc_send_error_response(ctx->request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR, "Internal error");

		}

		nvmf_rpc_remove_ns_ctx_free(ctx);

		return;

	}

}



static void

nvmf_rpc_remove_ns_paused(struct spdk_nvmf_subsystem *subsystem,

			  void *cb_arg, int status)

{

	struct nvmf_rpc_remove_ns_ctx *ctx = cb_arg;

	int ret;



	ret = spdk_nvmf_subsystem_remove_ns(subsystem, ctx->nsid, nvmf_rpc_remove_ns_remove_done, ctx);

	if (ret < 0) {

		SPDK_ERRLOG("Unable to remove namespace ID %u\n", ctx->nsid);

		spdk_jsonrpc_send_error_response(ctx->request, SPDK_JSONRPC_ERROR_INVALID_PARAMS,

						 "Invalid parameters");

		ctx->response_sent = true;

		spdk_nvmf_subsystem_resume(subsystem, nvmf_rpc_remove_ns_resumed, ctx);

	}

}



static void

spdk_rpc_nvmf_subsystem_remove_ns(struct spdk_jsonrpc_request *request,

				  const struct spdk_json_val *params)

lib/nvmf/subsystem.c

+10 −43
Original line number Diff line number Diff line
@@ -312,8 +312,6 @@ _spdk_nvmf_subsystem_remove_host(struct spdk_nvmf_subsystem *subsystem, struct s 
	free(host);

}



static int _spdk_nvmf_subsystem_remove_ns(struct spdk_nvmf_subsystem *subsystem, uint32_t nsid);



static void

_nvmf_subsystem_remove_listener(struct spdk_nvmf_subsystem *subsystem,

				struct spdk_nvmf_listener *listener)
@@ -361,7 +359,7 @@ spdk_nvmf_subsystem_destroy(struct spdk_nvmf_subsystem *subsystem) 
	while (ns != NULL) {

		struct spdk_nvmf_ns *next_ns = spdk_nvmf_subsystem_get_next_ns(subsystem, ns);



		_spdk_nvmf_subsystem_remove_ns(subsystem, ns->opts.nsid);

		spdk_nvmf_subsystem_remove_ns(subsystem, ns->opts.nsid);

		ns = next_ns;

	}
@@ -895,8 +893,8 @@ spdk_nvmf_subsystem_ns_changed(struct spdk_nvmf_subsystem *subsystem, uint32_t n 
	}

}



static int

_spdk_nvmf_subsystem_remove_ns(struct spdk_nvmf_subsystem *subsystem, uint32_t nsid)

int

spdk_nvmf_subsystem_remove_ns(struct spdk_nvmf_subsystem *subsystem, uint32_t nsid)

{

	struct spdk_nvmf_ns *ns;

	struct spdk_nvmf_registrant *reg, *reg_tmp;
@@ -936,50 +934,19 @@ _spdk_nvmf_subsystem_remove_ns(struct spdk_nvmf_subsystem *subsystem, uint32_t n 
	return 0;

}



int

spdk_nvmf_subsystem_remove_ns(struct spdk_nvmf_subsystem *subsystem, uint32_t nsid,

			      spdk_nvmf_subsystem_state_change_done cb_fn, void *cb_arg)

{

	int rc;

	struct subsystem_update_ns_ctx *ctx;



	rc = _spdk_nvmf_subsystem_remove_ns(subsystem, nsid);

	if (rc < 0) {

		return rc;

	}



	ctx = calloc(1, sizeof(*ctx));



	if (ctx == NULL) {

		return -ENOMEM;

	}



	ctx->subsystem = subsystem;

	ctx->cb_fn = cb_fn;

	ctx->cb_arg = cb_arg;



	spdk_nvmf_subsystem_update_ns(subsystem, subsystem_update_ns_done, ctx);



	return 0;

}



static void

_spdk_nvmf_ns_hot_remove_done(struct spdk_nvmf_subsystem *subsystem, void *cb_arg, int status)

{

	if (status != 0) {

		SPDK_ERRLOG("Failed to make changes to NVMe-oF subsystem with id %u\n", subsystem->id);

	}

	spdk_nvmf_subsystem_resume(subsystem, NULL, NULL);

}



static void

_spdk_nvmf_ns_hot_remove(struct spdk_nvmf_subsystem *subsystem,

			 void *cb_arg, int status)

{

	struct spdk_nvmf_ns *ns = cb_arg;

	int rc;



	spdk_nvmf_subsystem_remove_ns(subsystem, ns->opts.nsid, _spdk_nvmf_ns_hot_remove_done,

				      subsystem);

	rc = spdk_nvmf_subsystem_remove_ns(subsystem, ns->opts.nsid);

	if (rc != 0) {

		SPDK_ERRLOG("Failed to make changes to NVME-oF subsystem with id: %u\n", subsystem->id);

	}



	spdk_nvmf_subsystem_resume(subsystem, NULL, NULL);

}



static void

test/unit/lib/nvmf/subsystem.c/subsystem_ut.c

+5 −9
Original line number Diff line number Diff line
@@ -64,11 +64,6 @@ DEFINE_STUB(spdk_nvmf_transport_stop_listen, 
	    (struct spdk_nvmf_transport *transport,

	     const struct spdk_nvme_transport_id *trid), 0);



static void

subsystem_ns_remove_cb(struct spdk_nvmf_subsystem *subsystem, void *cb_arg, int status)

{

}



uint32_t

spdk_env_get_current_core(void)

{
@@ -255,6 +250,7 @@ test_spdk_nvmf_subsystem_add_ns(void) 
	struct spdk_bdev bdev1 = {}, bdev2 = {};

	struct spdk_nvmf_ns_opts ns_opts;

	uint32_t nsid;

	int rc;



	tgt.max_subsystems = 1024;

	tgt.subsystems = calloc(tgt.max_subsystems, sizeof(struct spdk_nvmf_subsystem *));
@@ -293,10 +289,10 @@ test_spdk_nvmf_subsystem_add_ns(void) 
	CU_ASSERT(nsid == 0);

	CU_ASSERT(subsystem.max_nsid == 5);



	spdk_nvmf_subsystem_remove_ns(&subsystem, 1, subsystem_ns_remove_cb, NULL);

	poll_threads();

	spdk_nvmf_subsystem_remove_ns(&subsystem, 5, subsystem_ns_remove_cb, NULL);

	poll_threads();

	rc = spdk_nvmf_subsystem_remove_ns(&subsystem, 1);

	CU_ASSERT(rc == 0);

	rc = spdk_nvmf_subsystem_remove_ns(&subsystem, 5);

	CU_ASSERT(rc == 0);



	free(subsystem.ns);

	free(tgt.subsystems);