Commit dd0d1a4c authored by Jacek Kalwas's avatar Jacek Kalwas Committed by Tomasz Zawadzki
Browse files

ut/bdev/nvme: cover ctrlr register vs aer race 



This check improves coverage for the situation where nvme_ctrlr_aer_cb
is called before the controller is registered as an I/O device.

A potential error can occur when the AER completion is received but the
initial ANA log page is not (see nvme_ctrlr_init_ana_log_page_done),
so this test simulates a similar scenario.

Without the fix
(bdev/nvme: prevent ns populate before ctrlr registration)
this test fails with assertion checking if callback is set. Without
assertion and actual AER callback being invoked it hits
> thread.c:2375:spdk_get_io_channel: *ERROR*: could not find io_device 0x513000009880
> bdev_nvme.c: 953:_bdev_nvme_add_io_path: *ERROR*: [nqn.2016-06.io.spdk:cnode1,192.168.100.9:4420,cntlid:31,nsid:1,ns:0x50b000001430,nbdev:0x517000005100] Failed to alloc io_channel.
> bdev_nvme.c:4999:bdev_nvme_add_io_path: *ERROR*: [nqn.2016-06.io.spdk:cnode1,192.168.100.9:4420,cntlid:31,nsid:1,ns:0x50b000001430,nbdev:0x517000005100] Failed to add I/O path to bdev_channel dynamically.

Change-Id: Id6f8cf4c5c1f1ae901a36abad2811c3ec627a860
Signed-off-by: default avatarJacek Kalwas <jacek.kalwas@nutanix.com>
Reviewed-on: https://review.spdk.io/c/spdk/spdk/+/26938


Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK Automated Test System <spdkbot@gmail.com>
Reviewed-by: default avatarShuhei Matsumoto <smatsumoto@nvidia.com>
Reviewed-by: default avatarTomasz Zawadzki <tomasz@tzawadzki.com>
parent 1a1e0743

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

+17 −6
Original line number Diff line number Diff line
@@ -138,9 +138,6 @@ DEFINE_STUB(spdk_nvme_ctrlr_get_max_xfer_size, uint32_t, 
DEFINE_STUB(spdk_nvme_ctrlr_get_transport_id, const struct spdk_nvme_transport_id *,

	    (struct spdk_nvme_ctrlr *ctrlr), NULL);



DEFINE_STUB_V(spdk_nvme_ctrlr_register_aer_callback, (struct spdk_nvme_ctrlr *ctrlr,

		spdk_nvme_aer_cb aer_cb_fn, void *aer_cb_arg));



DEFINE_STUB_V(spdk_nvme_ctrlr_register_timeout_callback, (struct spdk_nvme_ctrlr *ctrlr,

		uint64_t timeout_io_us, uint64_t timeout_admin_us, spdk_nvme_timeout_cb cb_fn, void *cb_arg));
@@ -328,6 +325,7 @@ struct spdk_nvme_ctrlr { 
	bool				is_failed;

	bool				fail_reset;

	bool				is_removed;

	spdk_nvme_aer_cb		aer_cb_fn;

	struct spdk_nvme_transport_id	trid;

	TAILQ_HEAD(, spdk_nvme_qpair)	active_io_qpairs;

	TAILQ_ENTRY(spdk_nvme_ctrlr)	tailq;
@@ -348,6 +346,14 @@ struct spdk_nvme_probe_ctx { 
	struct spdk_nvme_ctrlr		*init_ctrlr;

};



void

spdk_nvme_ctrlr_register_aer_callback(struct spdk_nvme_ctrlr *ctrlr,

				      spdk_nvme_aer_cb aer_cb_fn,

				      void *aer_cb_arg)

{

	ctrlr->aer_cb_fn = aer_cb_fn;

}



uint32_t

spdk_nvme_ctrlr_get_first_active_ns(struct spdk_nvme_ctrlr *ctrlr)

{
@@ -3972,12 +3978,17 @@ test_add_multi_io_paths_to_nbdev_ch(void) 
	spdk_delay_us(1000);

	poll_threads();



	spdk_delay_us(g_opts.nvme_adminq_poll_period_us);

	poll_threads();



	nvme_ctrlr3 = nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &path3.trid, opts.hostnqn);

	SPDK_CU_ASSERT_FATAL(nvme_ctrlr3 != NULL);



	/* AER should not be triggered before ctrlr registered as an io_device. */

	SPDK_CU_ASSERT_FATAL(ctrlr3->aer_cb_fn == NULL);



	/* During this call spdk_nvme_ctrlr_process_admin_completions is invoked. */

	spdk_delay_us(g_opts.nvme_adminq_poll_period_us);

	poll_threads();



	/* Namespace should be populated after nvme_ctrlr_init_ana_log_page_done and ctrlr registered as an io_device. */

	nvme_ns3 = _nvme_bdev_get_ns(nbdev, nvme_ctrlr3);

	SPDK_CU_ASSERT_FATAL(nvme_ns3 != NULL);