Commit eb387189 authored by Daniel Verkamp's avatar Daniel Verkamp
Browse files

nvmf: don't change NN while ctrlrs exist 



This was partially fixed in commit 1e481d04 ("nvmf: Do not allow NN
to change while connections present"), but we did not handle the case
where the user asked to add a NS with a NSID outside the current NN.

This patch reworks the logic (again) to be more straightforward and
hopefully more obviously correct.

Some confusion between max_allowed_nsid and max_nsid is also clarified;
if max_allowed_nsid is set, then max_nsid == max_allowed_nsid at all
times, so we don't need the extra logic when calculating NN in
spdk_nvmf_ctrlr_identify_ctrlr().

Change-Id: If531baf1114e03441ff3e1e1be098071702d9056
Signed-off-by: default avatarDaniel Verkamp <daniel.verkamp@intel.com>
Reviewed-on: https://review.gerrithub.io/414894


Tested-by: default avatarSPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
parent aceeb8cd

lib/nvmf/ctrlr.c

+1 −5
Original line number Diff line number Diff line
@@ -1296,11 +1296,7 @@ spdk_nvmf_ctrlr_identify_ctrlr(struct spdk_nvmf_ctrlr *ctrlr, struct spdk_nvme_c 
		cdata->sqes.max = 6;

		cdata->cqes.min = 4;

		cdata->cqes.max = 4;

		if (subsystem->max_allowed_nsid != 0) {

			cdata->nn = subsystem->max_allowed_nsid;

		} else {

		cdata->nn = subsystem->max_nsid;

		}

		cdata->vwc.present = 1;



		cdata->nvmf_specific.ioccsz = sizeof(struct spdk_nvme_cmd) / 16;

lib/nvmf/subsystem.c

+25 −30
Original line number Diff line number Diff line
@@ -973,7 +973,6 @@ spdk_nvmf_subsystem_add_ns(struct spdk_nvmf_subsystem *subsystem, struct spdk_bd 
{

	struct spdk_nvmf_ns_opts opts;

	struct spdk_nvmf_ns *ns;

	uint32_t i;

	int rc;



	if (!(subsystem->state == SPDK_NVMF_SUBSYSTEM_INACTIVE ||
@@ -995,44 +994,40 @@ spdk_nvmf_subsystem_add_ns(struct spdk_nvmf_subsystem *subsystem, struct spdk_bd 
		return 0;

	}



	if ((subsystem->max_allowed_nsid > 0) && (opts.nsid > subsystem->max_allowed_nsid)) {

		SPDK_ERRLOG("Can't extend NSID range above MaxNamespaces\n");

		return 0;

	}



	if (opts.nsid == 0) {

		/* NSID not specified - find a free index */

		for (i = 0; i < subsystem->max_nsid; i++) {

			if (_spdk_nvmf_subsystem_get_ns(subsystem, i + 1) == NULL) {

				opts.nsid = i + 1;

		/*

		 * NSID not specified - find a free index.

		 *

		 * If no free slots are found, opts.nsid will be subsystem->max_nsid + 1, which will

		 * expand max_nsid if possible.

		 */

		for (opts.nsid = 1; opts.nsid <= subsystem->max_nsid; opts.nsid++) {

			if (_spdk_nvmf_subsystem_get_ns(subsystem, opts.nsid) == NULL) {

				break;

			}

		}

		if (opts.nsid == 0) {

			/* All of the current slots are full */

			if (TAILQ_EMPTY(&subsystem->ctrlrs) &&

			    (subsystem->max_allowed_nsid == 0 ||

			     subsystem->max_nsid < subsystem->max_allowed_nsid)) {

				/* No controllers are connected and the subsystem is

				* not at its maximum NSID limit, so the array

				* can be expanded. */

				opts.nsid = subsystem->max_nsid + 1;

			} else {

				SPDK_ERRLOG("Can't extend NSID range above MaxNamespaces\n");

				return 0;

	}

		}

	} else {

		/* Specific NSID requested */



	if (_spdk_nvmf_subsystem_get_ns(subsystem, opts.nsid)) {

		SPDK_ERRLOG("Requested NSID %" PRIu32 " already in use\n", opts.nsid);

		return 0;

	}

	}



	if (opts.nsid > subsystem->max_nsid) {

		struct spdk_nvmf_ns **new_ns_array;



		/* If MaxNamespaces was specified, we can't extend max_nsid beyond it. */

		if (subsystem->max_allowed_nsid > 0 && opts.nsid > subsystem->max_allowed_nsid) {

			SPDK_ERRLOG("Can't extend NSID range above MaxNamespaces\n");

			return 0;

		}



		/* If a controller is connected, we can't change NN. */

		if (!TAILQ_EMPTY(&subsystem->ctrlrs)) {

			SPDK_ERRLOG("Can't extend NSID range while controllers are connected\n");

			return 0;

		}



		new_ns_array = realloc(subsystem->ns, sizeof(struct spdk_nvmf_ns *) * opts.nsid);

		if (new_ns_array == NULL) {

			SPDK_ERRLOG("Memory allocation error while resizing namespace array.\n");

test/nvmf/host/aer.sh

+3 −1
Original line number Diff line number Diff line
@@ -30,7 +30,8 @@ timing_exit start_nvmf_tgt 
modprobe -v nvme-rdma



$rpc_py construct_malloc_bdev 64 512 --name Malloc0

$rpc_py construct_nvmf_subsystem nqn.2016-06.io.spdk:cnode1 "trtype:RDMA traddr:$NVMF_FIRST_TARGET_IP trsvcid:$NVMF_PORT" '' -a -s SPDK00000000000001 -n Malloc0

$rpc_py construct_nvmf_subsystem nqn.2016-06.io.spdk:cnode1 "trtype:RDMA traddr:$NVMF_FIRST_TARGET_IP trsvcid:$NVMF_PORT" '' -a -s SPDK00000000000001 -n Malloc0 -m 2

$rpc_py get_nvmf_subsystems



# TODO: this aer test tries to invoke an AER completion by setting the temperature

#threshold to a very low value.  This does not work with emulated controllers
@@ -58,6 +59,7 @@ sleep 5 
# Add a new namespace

$rpc_py construct_malloc_bdev 64 4096 --name Malloc1

$rpc_py nvmf_subsystem_add_ns nqn.2016-06.io.spdk:cnode1 Malloc1 -n 2

$rpc_py get_nvmf_subsystems



wait $aerpid