bdev/nvme: Add failover trid to an existing ctrlr in the attach callback

Previously, we separated two cases, creating an new nvme_bdev_ctrlr
and adding an new path to an existing nvme_bdev_ctrlr as early as
possible.

However, we can merge creating an new connection between these two
cases. In this patch, connect_attach_cb() calls bdev_nvme_add_trid()
and then calls spdk_nvme_detach() if the nvme_bdev_ctrlr whose name is
ctx->base_name already exists.

This will simplify ANA multipath feature.

Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I2eec92dbf96e979690a1087c5c4fc306a29f15de
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/5329
Community-CI: Broadcom CI
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>

module/bdev/nvme/bdev_nvme.c

@@ -1783,15 +1783,13 @@ nvme_ctrlr_populate_namespaces_done(struct nvme_async_probe_ctx *ctx)
 }
 
 static int
-bdev_nvme_add_trid(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr, struct spdk_nvme_transport_id *trid)
+bdev_nvme_add_trid(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr, struct spdk_nvme_ctrlr *new_ctrlr,
+		   struct spdk_nvme_transport_id *trid)
 {
-	struct spdk_nvme_ctrlr		*new_ctrlr;
-	struct spdk_nvme_ctrlr_opts	opts;
 	uint32_t			i;
 	struct spdk_nvme_ns		*ns, *new_ns;
 	const struct spdk_nvme_ns_data	*ns_data, *new_ns_data;
 	struct nvme_bdev_ctrlr_trid	*new_trid;
-	int				rc = 0;
 
 	assert(nvme_bdev_ctrlr != NULL);
 
@@ -1817,18 +1815,8 @@ bdev_nvme_add_trid(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr, struct spdk_nvme_tra
 		}
 	}
 
-	spdk_nvme_ctrlr_get_default_ctrlr_opts(&opts, sizeof(opts));
-	opts.transport_retry_count = g_opts.retry_count;
-
-	new_ctrlr = spdk_nvme_connect(trid, &opts, sizeof(opts));
-
-	if (new_ctrlr == NULL) {
-		return -ENODEV;
-	}
-
 	if (spdk_nvme_ctrlr_get_num_ns(new_ctrlr) != nvme_bdev_ctrlr->num_ns) {
-		rc = -EINVAL;
+		return -EINVAL;
-		goto out;
 	}
 
 	for (i = 1; i <= nvme_bdev_ctrlr->num_ns; i++) {
@@ -1844,22 +1832,18 @@ bdev_nvme_add_trid(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr, struct spdk_nvme_tra
 		ns_data = spdk_nvme_ns_get_data(ns);
 		new_ns_data = spdk_nvme_ns_get_data(new_ns);
 		if (memcmp(ns_data->nguid, new_ns_data->nguid, sizeof(ns_data->nguid))) {
-			rc = -EINVAL;
+			return -EINVAL;
-			goto out;
 		}
 	}
 
 	new_trid = calloc(1, sizeof(*new_trid));
 	if (new_trid == NULL) {
-		rc = -ENOMEM;
+		return -ENOMEM;
-		goto out;
 	}
 	new_trid->trid = *trid;
 	TAILQ_INSERT_TAIL(&nvme_bdev_ctrlr->trids, new_trid, link);
 
-out:
+	return 0;
-	spdk_nvme_detach(new_ctrlr);
-	return rc;
 }
 
 static void
@@ -1875,6 +1859,20 @@ connect_attach_cb(void *cb_ctx, const struct spdk_nvme_transport_id *trid,
 
 	spdk_poller_unregister(&ctx->poller);
 
+	nvme_bdev_ctrlr = nvme_bdev_ctrlr_get_by_name(ctx->base_name);
+	if (nvme_bdev_ctrlr) {
+		/* This is the case that a secondary path is added to an existing
+		 * nvme_bdev_ctrlr for failover. After checking if it can access the same
+		 * namespaces as the primary path, it is disconnected until failover occurs.
+		 */
+		rc = bdev_nvme_add_trid(nvme_bdev_ctrlr, ctrlr, &ctx->trid);
+
+		spdk_nvme_detach(ctrlr);
+
+		populate_namespaces_cb(ctx, 0, rc);
+		return;
+	}
+
 	rc = nvme_bdev_ctrlr_create(ctrlr, ctx->base_name, &ctx->trid, ctx->prchk_flags);
 	if (rc) {
 		SPDK_ERRLOG("Failed to create new device\n");
@@ -1916,8 +1914,6 @@ bdev_nvme_create(struct spdk_nvme_transport_id *trid,
 {
 	struct nvme_probe_skip_entry	*entry, *tmp;
 	struct nvme_async_probe_ctx	*ctx;
-	struct nvme_bdev_ctrlr		*existing_ctrlr;
-	int				rc;
 
 	/* TODO expand this check to include both the host and target TRIDs.
 	 * Only if both are the same should we fail.
@@ -1939,18 +1935,6 @@ bdev_nvme_create(struct spdk_nvme_transport_id *trid,
 	ctx->prchk_flags = prchk_flags;
 	ctx->trid = *trid;
 
-	existing_ctrlr = nvme_bdev_ctrlr_get_by_name(base_name);
-	if (existing_ctrlr) {
-		rc = bdev_nvme_add_trid(existing_ctrlr, trid);
-		if (rc) {
-			free(ctx);
-			return rc;
-		}
-
-		nvme_ctrlr_populate_namespaces_done(ctx);
-		return 0;
-	}
-
 	if (trid->trtype == SPDK_NVME_TRANSPORT_PCIE) {
 		TAILQ_FOREACH_SAFE(entry, &g_skipped_nvme_ctrlrs, tailq, tmp) {
 			if (spdk_nvme_transport_id_compare(trid, &entry->trid) == 0) {