bdev/crypto: Open base bdev first using spdk_bdev_open_ext()

vbdev_crypto_claim() gets bdev name instead of bdev pointer
as a parameter, and open the corresponding base bdev first using
spdk_bdev_open_ext().

The purpose is to fix the race condition due to the time gap
between spdk_bdev_get_by_name() and spdk_bdev_open(). A bdev pointer
is valid only while the bdev is opened.

Resize event is not supported for now.

Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ia0e1ce2ce696f431bb26af94729931c3ffb9a9d0
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/4588
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>

module/bdev/crypto/vbdev_crypto.c

@@ -142,7 +142,7 @@ static void _complete_internal_io(struct spdk_bdev_io *bdev_io, bool success, vo
 static void _complete_internal_read(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg);
 static void _complete_internal_write(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg);
 static void vbdev_crypto_examine(struct spdk_bdev *bdev);
-static int vbdev_crypto_claim(struct spdk_bdev *bdev);
+static int vbdev_crypto_claim(const char *bdev_name);
 static void vbdev_crypto_submit_request(struct spdk_io_channel *ch, struct spdk_bdev_io *bdev_io);
 
 /* List of crypto_bdev names and their base bdevs via configuration file. */
@@ -1528,24 +1528,17 @@ create_crypto_disk(const char *bdev_name, const char *vbdev_name,
 		   const char *crypto_pmd, const char *key,
 		   const char *cipher, const char *key2)
 {
-	struct spdk_bdev *bdev = NULL;
+	int rc;
-	int rc = 0;
-
-	bdev = spdk_bdev_get_by_name(bdev_name);
 
 	rc = vbdev_crypto_insert_name(bdev_name, vbdev_name, crypto_pmd, key, cipher, key2);
 	if (rc) {
 		return rc;
 	}
 
-	if (!bdev) {
+	rc = vbdev_crypto_claim(bdev_name);
+	if (rc == -ENODEV) {
 		SPDK_NOTICELOG("vbdev creation deferred pending base bdev arrival\n");
-		return 0;
+		rc = 0;
-	}
-
-	rc = vbdev_crypto_claim(bdev);
-	if (rc) {
-		return rc;
 	}
 
 	return rc;
@@ -1713,12 +1706,10 @@ vbdev_crypto_get_spdk_running_config(FILE *fp)
 	fprintf(fp, "\n");
 }
 
-/* Called when the underlying base bdev goes away. */
 static void
-vbdev_crypto_examine_hotremove_cb(void *ctx)
+vbdev_crypto_base_bdev_hotremove_cb(struct spdk_bdev *bdev_find)
 {
 	struct vbdev_crypto *crypto_bdev, *tmp;
-	struct spdk_bdev *bdev_find = ctx;
 
 	TAILQ_FOREACH_SAFE(crypto_bdev, &g_vbdev_crypto, link, tmp) {
 		if (bdev_find == crypto_bdev->base_bdev) {
@@ -1727,6 +1718,21 @@ vbdev_crypto_examine_hotremove_cb(void *ctx)
 	}
 }
 
+/* Called when the underlying base bdev triggers asynchronous event such as bdev removal. */
+static void
+vbdev_crypto_base_bdev_event_cb(enum spdk_bdev_event_type type, struct spdk_bdev *bdev,
+				void *event_ctx)
+{
+	switch (type) {
+	case SPDK_BDEV_EVENT_REMOVE:
+		vbdev_crypto_base_bdev_hotremove_cb(bdev);
+		break;
+	default:
+		SPDK_NOTICELOG("Unsupported bdev event: type %d\n", type);
+		break;
+	}
+}
+
 static void
 vbdev_crypto_write_config_json(struct spdk_bdev *bdev, struct spdk_json_write_ctx *w)
 {
@@ -1756,11 +1762,12 @@ static struct spdk_bdev_module crypto_if = {
 SPDK_BDEV_MODULE_REGISTER(crypto, &crypto_if)
 
 static int
-vbdev_crypto_claim(struct spdk_bdev *bdev)
+vbdev_crypto_claim(const char *bdev_name)
 {
 	struct bdev_names *name;
 	struct vbdev_crypto *vbdev;
 	struct vbdev_dev *device;
+	struct spdk_bdev *bdev;
 	bool found = false;
 	int rc = 0;
 
@@ -1775,10 +1782,10 @@ vbdev_crypto_claim(struct spdk_bdev *bdev)
 	 * there's a match, create the crypto_bdev & bdev accordingly.
 	 */
 	TAILQ_FOREACH(name, &g_bdev_names, link) {
-		if (strcmp(name->bdev_name, bdev->name) != 0) {
+		if (strcmp(name->bdev_name, bdev_name) != 0) {
 			continue;
 		}
-		SPDK_DEBUGLOG(vbdev_crypto, "Match on %s\n", bdev->name);
+		SPDK_DEBUGLOG(vbdev_crypto, "Match on %s\n", bdev_name);
 
 		vbdev = calloc(1, sizeof(struct vbdev_crypto));
 		if (!vbdev) {
@@ -1787,8 +1794,6 @@ vbdev_crypto_claim(struct spdk_bdev *bdev)
 			goto error_vbdev_alloc;
 		}
 
-		/* The base bdev that we're attaching to. */
-		vbdev->base_bdev = bdev;
 		vbdev->crypto_bdev.name = strdup(name->vbdev_name);
 		if (!vbdev->crypto_bdev.name) {
 			SPDK_ERRLOG("could not allocate crypto_bdev name\n");
@@ -1820,6 +1825,19 @@ vbdev_crypto_claim(struct spdk_bdev *bdev)
 		}
 
 		vbdev->crypto_bdev.product_name = "crypto";
+
+		rc = spdk_bdev_open_ext(bdev_name, true, vbdev_crypto_base_bdev_event_cb,
+					NULL, &vbdev->base_desc);
+		if (rc) {
+			if (rc != -ENODEV) {
+				SPDK_ERRLOG("could not open bdev %s\n", bdev_name);
+			}
+			goto error_open;
+		}
+
+		bdev = spdk_bdev_desc_get_bdev(vbdev->base_desc);
+		vbdev->base_bdev = bdev;
+
 		vbdev->crypto_bdev.write_cache = bdev->write_cache;
 		vbdev->cipher = AES_CBC;
 		if (strcmp(vbdev->drv_name, QAT) == 0) {
@@ -1870,13 +1888,6 @@ vbdev_crypto_claim(struct spdk_bdev *bdev)
 		spdk_io_device_register(vbdev, crypto_bdev_ch_create_cb, crypto_bdev_ch_destroy_cb,
 					sizeof(struct crypto_io_channel), vbdev->crypto_bdev.name);
 
-		rc = spdk_bdev_open(bdev, true, vbdev_crypto_examine_hotremove_cb,
-				    bdev, &vbdev->base_desc);
-		if (rc) {
-			SPDK_ERRLOG("could not open bdev %s\n", spdk_bdev_get_name(bdev));
-			goto error_open;
-		}
-
 		/* Save the thread where the base device is opened */
 		vbdev->thread = spdk_get_thread();
 
@@ -1971,11 +1982,11 @@ error_session_en_create:
 error_cant_find_devid:
 error_claim:
 	spdk_bdev_close(vbdev->base_desc);
-error_open:
 	TAILQ_REMOVE(&g_vbdev_crypto, vbdev, link);
 	spdk_io_device_unregister(vbdev, NULL);
 	free(vbdev->xts_key);
 error_xts_key:
+error_open:
 	free(vbdev->drv_name);
 error_drv_name:
 	free(vbdev->key2);
@@ -2033,7 +2044,7 @@ delete_crypto_disk(struct spdk_bdev *bdev, spdk_delete_crypto_complete cb_fn,
 static void
 vbdev_crypto_examine(struct spdk_bdev *bdev)
 {
-	vbdev_crypto_claim(bdev);
+	vbdev_crypto_claim(spdk_bdev_get_name(bdev));
 	spdk_bdev_module_examine_done(&crypto_if);
 }
 

test/unit/lib/bdev/crypto.c/crypto_ut.c

@@ -160,9 +160,10 @@ DEFINE_STUB(spdk_bdev_get_buf_align, size_t, (const struct spdk_bdev *bdev), 64)
 DEFINE_STUB(spdk_bdev_get_io_channel, struct spdk_io_channel *, (struct spdk_bdev_desc *desc), 0);
 DEFINE_STUB_V(spdk_bdev_unregister, (struct spdk_bdev *bdev, spdk_bdev_unregister_cb cb_fn,
 				     void *cb_arg));
-DEFINE_STUB(spdk_bdev_open, int, (struct spdk_bdev *bdev, bool write,
+DEFINE_STUB(spdk_bdev_open_ext, int, (const char *bdev_name, bool write,
-				  spdk_bdev_remove_cb_t remove_cb,
+				      spdk_bdev_event_cb_t event_cb,
-				  void *remove_ctx, struct spdk_bdev_desc **_desc), 0);
+				      void *event_ctx, struct spdk_bdev_desc **_desc), 0);
+DEFINE_STUB(spdk_bdev_desc_get_bdev, struct spdk_bdev *, (struct spdk_bdev_desc *desc), NULL);
 DEFINE_STUB(spdk_bdev_module_claim_bdev, int, (struct spdk_bdev *bdev, struct spdk_bdev_desc *desc,
 		struct spdk_bdev_module *module), 0);
 DEFINE_STUB_V(spdk_bdev_module_examine_done, (struct spdk_bdev_module *module));