Commit 59f3cdac authored by Jim Harris's avatar Jim Harris
Browse files

nvmf: don't always update discovery log when adding hosts 



If a subsystem has no listeners, then there is no need
to update the discovery log when adding a host, or setting
a subsystem to allow all hosts.

This eliminates some unnecessary discovery log update
notifications, especially when setting 'allow any hosts'
on a subsystem immediately after it is created (and before
it has any listeners).

Update unit test to check the adding a host to a
subsystem without listeners does not rev the genctr.

Signed-off-by: default avatarJim Harris <james.r.harris@intel.com>
Change-Id: I63dab5df564269e574bb925890088f52063aa378
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10546


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarDong Yi <dongx.yi@intel.com>
Reviewed-by: default avatarJacek Kalwas <jacek.kalwas@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
parent 3867f83d

lib/nvmf/subsystem.c

+6 −2
Original line number Diff line number Diff line
@@ -837,7 +837,9 @@ spdk_nvmf_subsystem_add_host(struct spdk_nvmf_subsystem *subsystem, const char * 


	TAILQ_INSERT_HEAD(&subsystem->hosts, host, link);



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

		nvmf_update_discovery_log(subsystem->tgt, hostnqn);

	}



	pthread_mutex_unlock(&subsystem->mutex);
@@ -946,7 +948,9 @@ spdk_nvmf_subsystem_set_allow_any_host(struct spdk_nvmf_subsystem *subsystem, bo 
{

	pthread_mutex_lock(&subsystem->mutex);

	subsystem->flags.allow_any_host = allow_any_host;

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

		nvmf_update_discovery_log(subsystem->tgt, NULL);

	}

	pthread_mutex_unlock(&subsystem->mutex);



	return 0;

test/unit/lib/nvmf/ctrlr_discovery.c/ctrlr_discovery_ut.c

+11 −1
Original line number Diff line number Diff line
@@ -313,9 +313,19 @@ test_discovery_log(void) 
	/* Add one subsystem and verify that the discovery log contains it */

	subsystem = spdk_nvmf_subsystem_create(&tgt, "nqn.2016-06.io.spdk:subsystem1",

					       SPDK_NVMF_SUBTYPE_NVME, 0);

	subsystem->flags.allow_any_host = true;

	SPDK_CU_ASSERT_FATAL(subsystem != NULL);



	rc = spdk_nvmf_subsystem_add_host(subsystem, hostnqn);

	CU_ASSERT(rc == 0);



	/* Get only genctr (first field in the header) */

	memset(buffer, 0xCC, sizeof(buffer));

	disc_log = (struct spdk_nvmf_discovery_log_page *)buffer;

	nvmf_get_discovery_log_page(&tgt, hostnqn, &iov, 1, 0, sizeof(disc_log->genctr),

				    &trid);

	/* No listeners yet on new subsystem, so genctr should still be 0. */

	CU_ASSERT(disc_log->genctr == 0);



	test_gen_trid(&trid, SPDK_NVME_TRANSPORT_RDMA, SPDK_NVMF_ADRFAM_IPV4, "1234", "5678");

	spdk_nvmf_subsystem_add_listener(subsystem, &trid, _subsystem_add_listen_done, NULL);

	subsystem->state = SPDK_NVMF_SUBSYSTEM_ACTIVE;