Commit 9ce869c2 authored by Jinlong Chen's avatar Jinlong Chen Committed by Konrad Sztyber
Browse files

nvme: add spdk_nvme_probe*_ext variants that report attach failures



Currently, users cannot get any failure information if spdk_nvme_probe
or spdk_nvme_probe_async fails to attach probed devices.

This patch adds two functions, spdk_nvme_probe_ext and
spdk_nvme_probe_async_ext, that accept an extra attach_fail_cb parameter
compared to their existing counterparts. For each device that is probed
but unable to attach, attach_fail_cb will be called with the trid of the
device and an error code describing the failure reason to inform user
about the failure.

Fixes #3106

Change-Id: I55a69086b85987d9e0f5a31d1321fe3e006c2de7
Signed-off-by: default avatarJinlong Chen <chenjinlong.cjl@alibaba-inc.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/19872


Reviewed-by: default avatarGangCao <gang.cao@intel.com>
Reviewed-by: default avatarJim Harris <jim.harris@samsung.com>
Reviewed-by: default avatarKonrad Sztyber <konrad.sztyber@intel.com>
Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent 04f7c362
Loading
Loading
Loading
Loading
+75 −0
Original line number Diff line number Diff line
@@ -862,6 +862,17 @@ typedef void (*spdk_nvme_attach_cb)(void *cb_ctx, const struct spdk_nvme_transpo
				    struct spdk_nvme_ctrlr *ctrlr,
				    const struct spdk_nvme_ctrlr_opts *opts);

/**
 * Callback for spdk_nvme_probe*_ext() to report a device that has been probed but
 * unable to attach to the userspace NVMe driver.
 *
 * \param cb_ctx Opaque value passed to spdk_nvme_probe*_ext().
 * \param trid NVMe transport identifier.
 * \param rc Negative error code that provides information about the failure.
 */
typedef void (*spdk_nvme_attach_fail_cb)(void *cb_ctx, const struct spdk_nvme_transport_id *trid,
		int rc);

/**
 * Callback for spdk_nvme_remove() to report that a device attached to the userspace
 * NVMe driver has been removed from the system.
@@ -933,6 +944,37 @@ int spdk_nvme_probe(const struct spdk_nvme_transport_id *trid,
		    spdk_nvme_attach_cb attach_cb,
		    spdk_nvme_remove_cb remove_cb);

/**
 * Enumerate the bus indicated by the transport ID and attach the userspace NVMe
 * driver to each device found if desired.
 *
 * This works just the same as spdk_nvme_probe(), except that it calls attach_fail_cb
 * for devices that are probed but unabled to attach.
 *
 * \param trid The transport ID indicating which bus to enumerate. If the trtype
 * is PCIe or trid is NULL, this will scan the local PCIe bus. If the trtype is
 * fabrics (e.g. RDMA, TCP), the traddr and trsvcid must point at the location of an
 * NVMe-oF discovery service.
 * \param cb_ctx Opaque value which will be passed back in cb_ctx parameter of
 * the callbacks.
 * \param probe_cb will be called once per NVMe device found in the system.
 * \param attach_cb will be called for devices for which probe_cb returned true
 * once that NVMe controller has been attached to the userspace driver.
 * \param attach_fail_cb will be called for devices which probe_cb returned true
 * but failed to attach to the userspace driver.
 * \param remove_cb will be called for devices that were attached in a previous
 * spdk_nvme_probe() call but are no longer attached to the system. Optional;
 * specify NULL if removal notices are not desired.
 *
 * \return 0 on success, -1 on failure.
 */
int spdk_nvme_probe_ext(const struct spdk_nvme_transport_id *trid,
			void *cb_ctx,
			spdk_nvme_probe_cb probe_cb,
			spdk_nvme_attach_cb attach_cb,
			spdk_nvme_attach_fail_cb attach_fail_cb,
			spdk_nvme_remove_cb remove_cb);

/**
 * Connect the NVMe driver to the device located at the given transport ID.
 *
@@ -1020,6 +1062,39 @@ struct spdk_nvme_probe_ctx *spdk_nvme_probe_async(const struct spdk_nvme_transpo
		spdk_nvme_attach_cb attach_cb,
		spdk_nvme_remove_cb remove_cb);

/**
 * Probe and add controllers to the probe context list.
 *
 * Users must call spdk_nvme_probe_poll_async() to initialize
 * controllers in the probe context list to the READY state.
 *
 * This works just the same as spdk_nvme_probe_async(), except that it calls
 * attach_fail_cb for devices that are probed but unabled to attach.
 *
 * \param trid The transport ID indicating which bus to enumerate. If the trtype
 * is PCIe or trid is NULL, this will scan the local PCIe bus. If the trtype is
 * fabrics (e.g. RDMA, TCP), the traddr and trsvcid must point at the location of an
 * NVMe-oF discovery service.
 * \param cb_ctx Opaque value which will be passed back in cb_ctx parameter of
 * the callbacks.
 * \param probe_cb will be called once per NVMe device found in the system.
 * \param attach_cb will be called for devices for which probe_cb returned true
 * once that NVMe controller has been attached to the userspace driver.
 * \param attach_fail_cb will be called for devices which probe_cb returned true
 * but failed to attach to the userspace driver.
 * \param remove_cb will be called for devices that were attached in a previous
 * spdk_nvme_probe() call but are no longer attached to the system. Optional;
 * specify NULL if removal notices are not desired.
 *
 * \return probe context on success, NULL on failure.
 */
struct spdk_nvme_probe_ctx *spdk_nvme_probe_async_ext(const struct spdk_nvme_transport_id *trid,
		void *cb_ctx,
		spdk_nvme_probe_cb probe_cb,
		spdk_nvme_attach_cb attach_cb,
		spdk_nvme_attach_fail_cb attach_fail_cb,
		spdk_nvme_remove_cb remove_cb);

/**
 * Proceed with attaching controllers associated with the probe context.
 *
+45 −5
Original line number Diff line number Diff line
@@ -661,6 +661,7 @@ nvme_ctrlr_probe(const struct spdk_nvme_transport_id *trid,
				/* This ctrlr is being destructed asynchronously. */
				SPDK_ERRLOG("NVMe controller for SSD: %s is being destructed\n",
					    trid->traddr);
				probe_ctx->attach_fail_cb(probe_ctx->cb_ctx, trid, -EBUSY);
				return -EBUSY;
			}

@@ -679,6 +680,7 @@ nvme_ctrlr_probe(const struct spdk_nvme_transport_id *trid,
		ctrlr = nvme_transport_ctrlr_construct(trid, &opts, devhandle);
		if (ctrlr == NULL) {
			SPDK_ERRLOG("Failed to construct NVMe controller for SSD: %s\n", trid->traddr);
			probe_ctx->attach_fail_cb(probe_ctx->cb_ctx, trid, -ENODEV);
			return -1;
		}
		ctrlr->remove_cb = probe_ctx->remove_cb;
@@ -704,6 +706,7 @@ nvme_ctrlr_poll_internal(struct spdk_nvme_ctrlr *ctrlr,
		/* Controller failed to initialize. */
		TAILQ_REMOVE(&probe_ctx->init_ctrlrs, ctrlr, tailq);
		SPDK_ERRLOG("Failed to initialize SSD: %s\n", ctrlr->trid.traddr);
		probe_ctx->attach_fail_cb(probe_ctx->cb_ctx, &ctrlr->trid, rc);
		nvme_ctrlr_lock(ctrlr);
		nvme_ctrlr_fail(ctrlr, false);
		nvme_ctrlr_unlock(ctrlr);
@@ -828,6 +831,7 @@ nvme_probe_internal(struct spdk_nvme_probe_ctx *probe_ctx,
		SPDK_ERRLOG("NVMe ctrlr scan failed\n");
		TAILQ_FOREACH_SAFE(ctrlr, &probe_ctx->init_ctrlrs, tailq, ctrlr_tmp) {
			TAILQ_REMOVE(&probe_ctx->init_ctrlrs, ctrlr, tailq);
			probe_ctx->attach_fail_cb(probe_ctx->cb_ctx, &ctrlr->trid, -EFAULT);
			nvme_transport_ctrlr_destruct(ctrlr);
		}
		nvme_robust_mutex_unlock(&g_spdk_nvme_driver->lock);
@@ -873,6 +877,16 @@ nvme_probe_internal(struct spdk_nvme_probe_ctx *probe_ctx,
	return 0;
}

static void
nvme_dummy_attach_fail_cb(void *cb_ctx, const struct spdk_nvme_transport_id *trid,
			  int rc)
{
	SPDK_ERRLOG("Failed to attach nvme ctrlr: trtype=%s adrfam=%s traddr=%s trsvcid=%s "
		    "subnqn=%s, %s\n", spdk_nvme_transport_id_trtype_str(trid->trtype),
		    spdk_nvme_transport_id_adrfam_str(trid->adrfam), trid->traddr, trid->trsvcid,
		    trid->subnqn, spdk_strerror(-rc));
}

static void
nvme_probe_ctx_init(struct spdk_nvme_probe_ctx *probe_ctx,
		    const struct spdk_nvme_transport_id *trid,
@@ -880,6 +894,7 @@ nvme_probe_ctx_init(struct spdk_nvme_probe_ctx *probe_ctx,
		    void *cb_ctx,
		    spdk_nvme_probe_cb probe_cb,
		    spdk_nvme_attach_cb attach_cb,
		    spdk_nvme_attach_fail_cb attach_fail_cb,
		    spdk_nvme_remove_cb remove_cb)
{
	probe_ctx->trid = *trid;
@@ -887,6 +902,11 @@ nvme_probe_ctx_init(struct spdk_nvme_probe_ctx *probe_ctx,
	probe_ctx->cb_ctx = cb_ctx;
	probe_ctx->probe_cb = probe_cb;
	probe_ctx->attach_cb = attach_cb;
	if (attach_fail_cb != NULL) {
		probe_ctx->attach_fail_cb = attach_fail_cb;
	} else {
		probe_ctx->attach_fail_cb = nvme_dummy_attach_fail_cb;
	}
	probe_ctx->remove_cb = remove_cb;
	TAILQ_INIT(&probe_ctx->init_ctrlrs);
}
@@ -895,6 +915,14 @@ int
spdk_nvme_probe(const struct spdk_nvme_transport_id *trid, void *cb_ctx,
		spdk_nvme_probe_cb probe_cb, spdk_nvme_attach_cb attach_cb,
		spdk_nvme_remove_cb remove_cb)
{
	return spdk_nvme_probe_ext(trid, cb_ctx, probe_cb, attach_cb, NULL, remove_cb);
}

int
spdk_nvme_probe_ext(const struct spdk_nvme_transport_id *trid, void *cb_ctx,
		    spdk_nvme_probe_cb probe_cb, spdk_nvme_attach_cb attach_cb,
		    spdk_nvme_attach_fail_cb attach_fail_cb, spdk_nvme_remove_cb remove_cb)
{
	struct spdk_nvme_transport_id trid_pcie;
	struct spdk_nvme_probe_ctx *probe_ctx;
@@ -905,8 +933,8 @@ spdk_nvme_probe(const struct spdk_nvme_transport_id *trid, void *cb_ctx,
		trid = &trid_pcie;
	}

	probe_ctx = spdk_nvme_probe_async(trid, cb_ctx, probe_cb,
					  attach_cb, remove_cb);
	probe_ctx = spdk_nvme_probe_async_ext(trid, cb_ctx, probe_cb,
					      attach_cb, attach_fail_cb, remove_cb);
	if (!probe_ctx) {
		SPDK_ERRLOG("Create probe context failed\n");
		return -1;
@@ -1518,7 +1546,7 @@ spdk_nvme_scan_attached(const struct spdk_nvme_transport_id *trid)
		return -ENOMEM;
	}

	nvme_probe_ctx_init(probe_ctx, trid, NULL, NULL, NULL, NULL, NULL);
	nvme_probe_ctx_init(probe_ctx, trid, NULL, NULL, NULL, NULL, NULL, NULL);

	nvme_robust_mutex_lock(&g_spdk_nvme_driver->lock);
	rc = nvme_transport_ctrlr_scan_attached(probe_ctx);
@@ -1534,6 +1562,17 @@ spdk_nvme_probe_async(const struct spdk_nvme_transport_id *trid,
		      spdk_nvme_probe_cb probe_cb,
		      spdk_nvme_attach_cb attach_cb,
		      spdk_nvme_remove_cb remove_cb)
{
	return spdk_nvme_probe_async_ext(trid, cb_ctx, probe_cb, attach_cb, NULL, remove_cb);
}

struct spdk_nvme_probe_ctx *
spdk_nvme_probe_async_ext(const struct spdk_nvme_transport_id *trid,
			  void *cb_ctx,
			  spdk_nvme_probe_cb probe_cb,
			  spdk_nvme_attach_cb attach_cb,
			  spdk_nvme_attach_fail_cb attach_fail_cb,
			  spdk_nvme_remove_cb remove_cb)
{
	int rc;
	struct spdk_nvme_probe_ctx *probe_ctx;
@@ -1548,7 +1587,8 @@ spdk_nvme_probe_async(const struct spdk_nvme_transport_id *trid,
		return NULL;
	}

	nvme_probe_ctx_init(probe_ctx, trid, NULL, cb_ctx, probe_cb, attach_cb, remove_cb);
	nvme_probe_ctx_init(probe_ctx, trid, NULL, cb_ctx, probe_cb, attach_cb, attach_fail_cb,
			    remove_cb);
	rc = nvme_probe_internal(probe_ctx, false);
	if (rc != 0) {
		free(probe_ctx);
@@ -1606,7 +1646,7 @@ spdk_nvme_connect_async(const struct spdk_nvme_transport_id *trid,
		probe_cb = nvme_connect_probe_cb;
	}

	nvme_probe_ctx_init(probe_ctx, trid, opts, (void *)opts, probe_cb, attach_cb, NULL);
	nvme_probe_ctx_init(probe_ctx, trid, opts, (void *)opts, probe_cb, attach_cb, NULL, NULL);
	rc = nvme_probe_internal(probe_ctx, true);
	if (rc != 0) {
		free(probe_ctx);
+1 −0
Original line number Diff line number Diff line
@@ -1107,6 +1107,7 @@ struct spdk_nvme_probe_ctx {
	void					*cb_ctx;
	spdk_nvme_probe_cb			probe_cb;
	spdk_nvme_attach_cb			attach_cb;
	spdk_nvme_attach_fail_cb		attach_fail_cb;
	spdk_nvme_remove_cb			remove_cb;
	TAILQ_HEAD(, spdk_nvme_ctrlr)		init_ctrlrs;
};
+2 −0
Original line number Diff line number Diff line
@@ -21,9 +21,11 @@
	spdk_nvme_prchk_flags_str;

	spdk_nvme_probe;
	spdk_nvme_probe_ext;
	spdk_nvme_connect;
	spdk_nvme_connect_async;
	spdk_nvme_probe_async;
	spdk_nvme_probe_async_ext;
	spdk_nvme_probe_poll_async;
	spdk_nvme_detach;
	spdk_nvme_detach_async;
+44 −11
Original line number Diff line number Diff line
@@ -164,14 +164,22 @@ dummy_attach_cb(void *cb_ctx, const struct spdk_nvme_transport_id *trid,
	ut_attach_cb_called = true;
}

static int ut_attach_fail_cb_rc = 0;
static void
test_spdk_nvme_probe(void)
dummy_attach_fail_cb(void *cb_ctx, const struct spdk_nvme_transport_id *trid, int rc)
{
	ut_attach_fail_cb_rc = rc;
}

static void
test_spdk_nvme_probe_ext(void)
{
	int rc = 0;
	const struct spdk_nvme_transport_id *trid = NULL;
	void *cb_ctx = NULL;
	spdk_nvme_probe_cb probe_cb = NULL;
	spdk_nvme_attach_cb attach_cb = dummy_attach_cb;
	spdk_nvme_attach_fail_cb attach_fail_cb = dummy_attach_fail_cb;
	spdk_nvme_remove_cb remove_cb = NULL;
	struct spdk_nvme_ctrlr ctrlr;
	pthread_mutexattr_t attr;
@@ -181,7 +189,7 @@ test_spdk_nvme_probe(void)
	/* driver init fails */
	MOCK_SET(spdk_process_is_primary, false);
	MOCK_SET(spdk_memzone_lookup, NULL);
	rc = spdk_nvme_probe(trid, cb_ctx, probe_cb, attach_cb, remove_cb);
	rc = spdk_nvme_probe_ext(trid, cb_ctx, probe_cb, attach_cb, attach_fail_cb, remove_cb);
	CU_ASSERT(rc == -1);

	/*
@@ -193,7 +201,7 @@ test_spdk_nvme_probe(void)
	MOCK_SET(spdk_process_is_primary, true);
	dummy.initialized = true;
	g_spdk_nvme_driver = &dummy;
	rc = spdk_nvme_probe(trid, cb_ctx, probe_cb, attach_cb, remove_cb);
	rc = spdk_nvme_probe_ext(trid, cb_ctx, probe_cb, attach_cb, attach_fail_cb, remove_cb);
	CU_ASSERT(rc == -1);

	/* driver init passes, transport available, secondary call attach_cb */
@@ -209,13 +217,13 @@ test_spdk_nvme_probe(void)
	ut_attach_cb_called = false;
	/* setup nvme_transport_ctrlr_scan() stub to also check the trype */
	ut_check_trtype = true;
	rc = spdk_nvme_probe(trid, cb_ctx, probe_cb, attach_cb, remove_cb);
	rc = spdk_nvme_probe_ext(trid, cb_ctx, probe_cb, attach_cb, attach_fail_cb, remove_cb);
	CU_ASSERT(rc == 0);
	CU_ASSERT(ut_attach_cb_called == true);

	/* driver init passes, transport available, we are primary */
	MOCK_SET(spdk_process_is_primary, true);
	rc = spdk_nvme_probe(trid, cb_ctx, probe_cb, attach_cb, remove_cb);
	rc = spdk_nvme_probe_ext(trid, cb_ctx, probe_cb, attach_cb, attach_fail_cb, remove_cb);
	CU_ASSERT(rc == 0);

	g_spdk_nvme_driver = NULL;
@@ -347,6 +355,7 @@ test_nvme_init_controllers(void)
	struct nvme_driver test_driver = {};
	void *cb_ctx = NULL;
	spdk_nvme_attach_cb attach_cb = dummy_attach_cb;
	spdk_nvme_attach_fail_cb attach_fail_cb = dummy_attach_fail_cb;
	struct spdk_nvme_probe_ctx *probe_ctx;
	struct spdk_nvme_ctrlr *ctrlr;
	pthread_mutexattr_t attr;
@@ -368,15 +377,18 @@ test_nvme_init_controllers(void)
	MOCK_SET(spdk_process_is_primary, 1);
	g_spdk_nvme_driver->initialized = false;
	ut_destruct_called = false;
	ut_attach_fail_cb_rc = 0;
	probe_ctx = test_nvme_init_get_probe_ctx();
	TAILQ_INSERT_TAIL(&probe_ctx->init_ctrlrs, ctrlr, tailq);
	probe_ctx->cb_ctx = cb_ctx;
	probe_ctx->attach_cb = attach_cb;
	probe_ctx->attach_fail_cb = attach_fail_cb;
	probe_ctx->trid.trtype = SPDK_NVME_TRANSPORT_PCIE;
	rc = nvme_init_controllers(probe_ctx);
	CU_ASSERT(rc == 0);
	CU_ASSERT(g_spdk_nvme_driver->initialized == true);
	CU_ASSERT(ut_destruct_called == true);
	CU_ASSERT(ut_attach_fail_cb_rc == 1);

	/*
	 * Controller init OK, need to move the controller state machine
@@ -832,29 +844,47 @@ test_nvme_ctrlr_probe(void)
	void *cb_ctx = NULL;
	struct spdk_nvme_ctrlr *dummy = NULL;

	nvme_driver_init();

	ctrlr.adminq = &qpair;
	nvme_get_default_hostnqn(ctrlr.opts.hostnqn, sizeof(ctrlr.opts.hostnqn));

	TAILQ_INIT(&probe_ctx.init_ctrlrs);
	nvme_driver_init();

	/* test when probe_cb returns false */

	MOCK_SET(dummy_probe_cb, false);
	nvme_probe_ctx_init(&probe_ctx, &trid, NULL, cb_ctx, dummy_probe_cb, NULL, NULL);
	nvme_probe_ctx_init(&probe_ctx, &trid, NULL, cb_ctx, dummy_probe_cb, NULL, NULL, NULL);
	rc = nvme_ctrlr_probe(&trid, &probe_ctx, devhandle);
	CU_ASSERT(rc == 1);

	/* probe_cb returns true but we find a destructing ctrlr */
	MOCK_SET(dummy_probe_cb, true);
	ut_attach_fail_cb_rc = 0;
	ctrlr.is_destructed = true;
	TAILQ_INSERT_TAIL(&g_nvme_attached_ctrlrs, &ctrlr, tailq);
	nvme_probe_ctx_init(&probe_ctx, &trid, NULL, cb_ctx, dummy_probe_cb, NULL,
			    dummy_attach_fail_cb, NULL);
	rc = nvme_ctrlr_probe(&trid, &probe_ctx, devhandle);
	CU_ASSERT(rc == -EBUSY);
	CU_ASSERT(ut_attach_fail_cb_rc == -EBUSY);
	TAILQ_REMOVE(&g_nvme_attached_ctrlrs, &ctrlr, tailq);
	ctrlr.is_destructed = false;

	/* probe_cb returns true but we can't construct a ctrl */
	MOCK_SET(dummy_probe_cb, true);
	MOCK_SET(nvme_transport_ctrlr_construct, NULL);
	nvme_probe_ctx_init(&probe_ctx, &trid, NULL, cb_ctx, dummy_probe_cb, NULL, NULL);
	ut_attach_fail_cb_rc = 0;
	nvme_probe_ctx_init(&probe_ctx, &trid, NULL, cb_ctx, dummy_probe_cb, NULL,
			    dummy_attach_fail_cb, NULL);
	rc = nvme_ctrlr_probe(&trid, &probe_ctx, devhandle);
	CU_ASSERT(rc == -1);
	CU_ASSERT(ut_attach_fail_cb_rc == -ENODEV);

	/* happy path */
	MOCK_SET(dummy_probe_cb, true);
	MOCK_SET(nvme_transport_ctrlr_construct, &ctrlr);
	nvme_probe_ctx_init(&probe_ctx, &trid, NULL, cb_ctx, dummy_probe_cb, NULL, NULL);
	nvme_probe_ctx_init(&probe_ctx, &trid, NULL, cb_ctx, dummy_probe_cb, NULL, NULL, NULL);
	rc = nvme_ctrlr_probe(&trid, &probe_ctx, devhandle);
	CU_ASSERT(rc == 0);
	dummy = TAILQ_FIRST(&probe_ctx.init_ctrlrs);
@@ -1424,13 +1454,16 @@ test_nvme_ctrlr_probe_internal(void)
	rc = nvme_driver_init();
	CU_ASSERT(rc == 0);

	ut_attach_fail_cb_rc = 0;
	ut_test_probe_internal = true;
	MOCK_SET(dummy_probe_cb, true);
	trid.trtype = SPDK_NVME_TRANSPORT_PCIE;
	nvme_probe_ctx_init(probe_ctx, &trid, NULL, NULL, dummy_probe_cb, NULL, NULL);
	nvme_probe_ctx_init(probe_ctx, &trid, NULL, NULL, dummy_probe_cb, NULL, dummy_attach_fail_cb,
			    NULL);
	rc = nvme_probe_internal(probe_ctx, false);
	CU_ASSERT(rc < 0);
	CU_ASSERT(TAILQ_EMPTY(&probe_ctx->init_ctrlrs));
	CU_ASSERT(ut_attach_fail_cb_rc == -EFAULT);

	free(probe_ctx);
	ut_test_probe_internal = false;
@@ -1647,7 +1680,7 @@ main(int argc, char **argv)
	CU_ADD_TEST(suite, test_trid_trtype_str);
	CU_ADD_TEST(suite, test_trid_adrfam_str);
	CU_ADD_TEST(suite, test_nvme_ctrlr_probe);
	CU_ADD_TEST(suite, test_spdk_nvme_probe);
	CU_ADD_TEST(suite, test_spdk_nvme_probe_ext);
	CU_ADD_TEST(suite, test_spdk_nvme_connect);
	CU_ADD_TEST(suite, test_nvme_ctrlr_probe_internal);
	CU_ADD_TEST(suite, test_nvme_init_controllers);