diff --git a/include/spdk_internal/bdev.h b/include/spdk_internal/bdev.h index 19eb63614..83d1a731d 100644 --- a/include/spdk_internal/bdev.h +++ b/include/spdk_internal/bdev.h @@ -224,7 +224,7 @@ struct spdk_bdev { bool bdev_opened_for_write; - uint32_t vbdevs_opened_for_write; + struct spdk_bdev_module_if *vbdev_claim_module; /** List of open descriptors for this block device. */ TAILQ_HEAD(, spdk_bdev_desc) open_descs; @@ -385,6 +385,10 @@ void spdk_vbdev_unregister(struct spdk_bdev *vbdev); void spdk_vbdev_module_examine_done(struct spdk_bdev_module_if *module); +int spdk_vbdev_module_claim_bdev(struct spdk_bdev *bdev, struct spdk_bdev_desc *desc, + struct spdk_bdev_module_if *module); +void spdk_vbdev_module_release_bdev(struct spdk_bdev *bdev); + void spdk_bdev_poller_start(struct spdk_bdev_poller **ppoller, spdk_bdev_poller_fn fn, void *arg, diff --git a/lib/bdev/bdev.c b/lib/bdev/bdev.c index d44ed3284..a8ae347a7 100644 --- a/lib/bdev/bdev.c +++ b/lib/bdev/bdev.c @@ -1397,7 +1397,6 @@ _spdk_bdev_register(struct spdk_bdev *bdev) bdev->gencnt = 0; TAILQ_INIT(&bdev->open_descs); bdev->bdev_opened_for_write = false; - bdev->vbdevs_opened_for_write = 0; TAILQ_INIT(&bdev->vbdevs); TAILQ_INIT(&bdev->base_bdevs); @@ -1516,35 +1515,6 @@ spdk_vbdev_module_examine_done(struct spdk_bdev_module_if *module) } } -static bool -__is_bdev_opened_for_write(struct spdk_bdev *bdev) -{ - struct spdk_bdev *base; - - if (bdev->bdev_opened_for_write) { - return true; - } - - TAILQ_FOREACH(base, &bdev->base_bdevs, base_bdev_link) { - if (__is_bdev_opened_for_write(base)) { - return true; - } - } - - return false; -} - -static void -__modify_write_counts(struct spdk_bdev *bdev, int mod) -{ - struct spdk_bdev *base; - - TAILQ_FOREACH(base, &bdev->base_bdevs, base_bdev_link) { - base->vbdevs_opened_for_write += mod; - __modify_write_counts(base, mod); - } -} - int spdk_bdev_open(struct spdk_bdev *bdev, bool write, spdk_bdev_remove_cb_t remove_cb, void *remove_ctx, struct spdk_bdev_desc **_desc) @@ -1558,8 +1528,8 @@ spdk_bdev_open(struct spdk_bdev *bdev, bool write, spdk_bdev_remove_cb_t remove_ pthread_mutex_lock(&bdev->mutex); - if (write && (__is_bdev_opened_for_write(bdev) || bdev->vbdevs_opened_for_write > 0)) { - SPDK_ERRLOG("failed, %s (or one of its virtual bdevs) already opened for write\n", bdev->name); + if (write && (bdev->bdev_opened_for_write || bdev->vbdev_claim_module)) { + SPDK_ERRLOG("failed, %s already opened for write or claimed\n", bdev->name); free(desc); pthread_mutex_unlock(&bdev->mutex); return -EPERM; @@ -1569,7 +1539,6 @@ spdk_bdev_open(struct spdk_bdev *bdev, bool write, spdk_bdev_remove_cb_t remove_ if (write) { bdev->bdev_opened_for_write = true; - __modify_write_counts(bdev, 1); } desc->bdev = bdev; @@ -1594,7 +1563,6 @@ spdk_bdev_close(struct spdk_bdev_desc *desc) if (desc->write) { assert(bdev->bdev_opened_for_write); bdev->bdev_opened_for_write = false; - __modify_write_counts(bdev, -1); } TAILQ_REMOVE(&bdev->open_descs, desc, link); @@ -1610,6 +1578,36 @@ spdk_bdev_close(struct spdk_bdev_desc *desc) } } +int +spdk_vbdev_module_claim_bdev(struct spdk_bdev *bdev, struct spdk_bdev_desc *desc, + struct spdk_bdev_module_if *module) +{ + if (bdev->vbdev_claim_module != NULL) { + SPDK_ERRLOG("bdev %s already claimed by module %s\n", bdev->name, + bdev->vbdev_claim_module->name); + return -EPERM; + } + + if ((!desc || !desc->write) && bdev->bdev_opened_for_write) { + SPDK_ERRLOG("bdev %s already opened with write access\n", bdev->name); + return -EPERM; + } + + if (desc && !desc->write) { + desc->write = true; + } + + bdev->vbdev_claim_module = module; + return 0; +} + +void +spdk_vbdev_module_release_bdev(struct spdk_bdev *bdev) +{ + assert(bdev->vbdev_claim_module != NULL); + bdev->vbdev_claim_module = NULL; +} + void spdk_bdev_io_get_iovec(struct spdk_bdev_io *bdev_io, struct iovec **iovp, int *iovcntp) { diff --git a/lib/bdev/error/vbdev_error.c b/lib/bdev/error/vbdev_error.c index 00ededbdf..24aae4da6 100644 --- a/lib/bdev/error/vbdev_error.c +++ b/lib/bdev/error/vbdev_error.c @@ -259,13 +259,22 @@ spdk_vbdev_error_create(struct spdk_bdev *base_bdev) goto cleanup; } + rc = spdk_vbdev_module_claim_bdev(base_bdev, NULL, SPDK_GET_BDEV_MODULE(error)); + if (rc) { + SPDK_ERRLOG("could not claim bdev %s\n", spdk_bdev_get_name(base_bdev)); + goto cleanup; + } + disk->base_bdev = base_bdev; - memcpy(&disk->disk, base_bdev, sizeof(*base_bdev)); disk->disk.name = spdk_sprintf_alloc("EE_%s", base_bdev->name); if (!disk->disk.name) { rc = -ENOMEM; goto cleanup; } + disk->disk.blockcnt = base_bdev->blockcnt; + disk->disk.blocklen = base_bdev->blocklen; + disk->disk.write_cache = base_bdev->write_cache; + disk->disk.max_unmap_bdesc_count = base_bdev->max_unmap_bdesc_count; disk->disk.product_name = "Error Injection Disk"; disk->disk.ctxt = disk; disk->disk.fn_table = &vbdev_error_fn_table; diff --git a/lib/bdev/gpt/vbdev_gpt.c b/lib/bdev/gpt/vbdev_gpt.c index d8812a2d8..5a12073eb 100644 --- a/lib/bdev/gpt/vbdev_gpt.c +++ b/lib/bdev/gpt/vbdev_gpt.c @@ -422,6 +422,8 @@ static void spdk_gpt_bdev_complete(struct spdk_bdev_io *bdev_io, bool status, void *arg) { struct spdk_gpt_bdev *gpt_bdev = (struct spdk_gpt_bdev *)arg; + struct spdk_bdev *bdev = gpt_bdev->bdev; + bool claimed = false; int rc; /* free the ch and also close the bdev_desc */ @@ -429,10 +431,11 @@ spdk_gpt_bdev_complete(struct spdk_bdev_io *bdev_io, bool status, void *arg) gpt_bdev->ch = NULL; spdk_bdev_close(gpt_bdev->bdev_desc); gpt_bdev->bdev_desc = NULL; + spdk_bdev_free_io(bdev_io); if (status != SPDK_BDEV_IO_STATUS_SUCCESS) { SPDK_ERRLOG("Gpt: bdev=%s io error status=%d\n", - spdk_bdev_get_name(gpt_bdev->bdev), status); + spdk_bdev_get_name(bdev), status); goto end; } @@ -442,10 +445,17 @@ spdk_gpt_bdev_complete(struct spdk_bdev_io *bdev_io, bool status, void *arg) goto end; } + rc = spdk_vbdev_module_claim_bdev(bdev, NULL, SPDK_GET_BDEV_MODULE(gpt)); + if (rc) { + SPDK_ERRLOG("could not claim bdev %s\n", spdk_bdev_get_name(bdev)); + goto end; + } + + claimed = true; rc = vbdev_gpt_create_bdevs(gpt_bdev); if (rc < 0) { SPDK_TRACELOG(SPDK_TRACE_VBDEV_GPT, "Failed to split dev=%s by gpt table\n", - spdk_bdev_get_name(gpt_bdev->bdev)); + spdk_bdev_get_name(bdev)); } end: @@ -455,10 +465,12 @@ end: */ spdk_vbdev_module_examine_done(SPDK_GET_BDEV_MODULE(gpt)); - spdk_bdev_free_io(bdev_io); if (gpt_bdev->ref == 0) { /* If no gpt_partition_disk instances were created, free the base context */ spdk_gpt_bdev_free(gpt_bdev); + if (claimed) { + spdk_vbdev_module_release_bdev(bdev); + } } } diff --git a/lib/bdev/rpc/bdev_rpc.c b/lib/bdev/rpc/bdev_rpc.c index 927b7c31e..ae940abf0 100644 --- a/lib/bdev/rpc/bdev_rpc.c +++ b/lib/bdev/rpc/bdev_rpc.c @@ -74,9 +74,6 @@ spdk_rpc_get_bdevs(struct spdk_jsonrpc_request *request, spdk_json_write_name(w, "bdev_opened_for_write"); spdk_json_write_bool(w, bdev->bdev_opened_for_write); - spdk_json_write_name(w, "vbdevs_opened_for_write"); - spdk_json_write_uint32(w, bdev->vbdevs_opened_for_write); - spdk_json_write_name(w, "driver_specific"); spdk_json_write_object_begin(w); spdk_bdev_dump_config_json(bdev, w); diff --git a/lib/bdev/split/vbdev_split.c b/lib/bdev/split/vbdev_split.c index c85662c55..33a8abd5f 100644 --- a/lib/bdev/split/vbdev_split.c +++ b/lib/bdev/split/vbdev_split.c @@ -240,6 +240,12 @@ vbdev_split_create(struct spdk_bdev *base_bdev, uint64_t split_count, uint64_t s int rc; struct split_base *split_base; + rc = spdk_vbdev_module_claim_bdev(base_bdev, NULL, SPDK_GET_BDEV_MODULE(split)); + if (rc) { + SPDK_ERRLOG("could not claim bdev %s\n", spdk_bdev_get_name(base_bdev)); + return -1; + } + if (split_size_mb) { if (((split_size_mb * mb) % base_bdev->blocklen) != 0) { SPDK_ERRLOG("Split size %" PRIu64 " MB is not possible with block size " diff --git a/test/lib/bdev/blockdev.sh b/test/lib/bdev/blockdev.sh index 508b67407..c50b84ab2 100755 --- a/test/lib/bdev/blockdev.sh +++ b/test/lib/bdev/blockdev.sh @@ -20,6 +20,8 @@ timing_exit bounds if [ $(uname -s) = Linux ] && hash sgdisk; then echo "[Rpc]" >> $testdir/bdev.conf echo " Enable Yes" >> $testdir/bdev.conf + echo "[Gpt]" >> $testdir/bdev.conf + echo " Disable Yes" >> $testdir/bdev.conf if [ ! -z "`grep "Nvme0" $testdir/bdev.conf`" ]; then modprobe nbd @@ -38,8 +40,10 @@ if [ $(uname -s) = Linux ] && hash sgdisk; then fi killprocess $nbd_pid - # run nbd again to test get_bdevs - $testdir/nbd/nbd -c $testdir/bdev.conf -b Nvme0n1 -n /dev/nbd0 & + # enable the gpt module and run nbd again to test get_bdevs and + # bind nbd to the new gpt partition bdev + sed -i'' '/Disable/d' $testdir/bdev.conf + $testdir/nbd/nbd -c $testdir/bdev.conf -b Nvme0n1p1 -n /dev/nbd0 & nbd_pid=$! waitforlisten $nbd_pid 5260 waitforbdev Nvme0n1p1 $rootdir/scripts/rpc.py diff --git a/test/unit/lib/bdev/bdev.c/bdev_ut.c b/test/unit/lib/bdev/bdev.c/bdev_ut.c index fe6cf5a01..323018a22 100644 --- a/test/unit/lib/bdev/bdev.c/bdev_ut.c +++ b/test/unit/lib/bdev/bdev.c/bdev_ut.c @@ -217,12 +217,25 @@ open_write_test(void) */ bdev[0] = allocate_bdev("bdev0"); + rc = spdk_vbdev_module_claim_bdev(bdev[0], NULL, SPDK_GET_BDEV_MODULE(bdev_ut)); + CU_ASSERT(rc == 0); + bdev[1] = allocate_bdev("bdev1"); + rc = spdk_vbdev_module_claim_bdev(bdev[1], NULL, SPDK_GET_BDEV_MODULE(bdev_ut)); + CU_ASSERT(rc == 0); + bdev[2] = allocate_bdev("bdev2"); + rc = spdk_vbdev_module_claim_bdev(bdev[2], NULL, SPDK_GET_BDEV_MODULE(bdev_ut)); + CU_ASSERT(rc == 0); bdev[3] = allocate_vbdev("bdev3", bdev[0], bdev[1]); + rc = spdk_vbdev_module_claim_bdev(bdev[3], NULL, SPDK_GET_BDEV_MODULE(bdev_ut)); + CU_ASSERT(rc == 0); bdev[4] = allocate_vbdev("bdev4", bdev[2], NULL); + rc = spdk_vbdev_module_claim_bdev(bdev[4], NULL, SPDK_GET_BDEV_MODULE(bdev_ut)); + CU_ASSERT(rc == 0); + bdev[5] = allocate_vbdev("bdev5", bdev[2], NULL); bdev[6] = allocate_vbdev("bdev6", bdev[2], NULL); @@ -234,14 +247,16 @@ open_write_test(void) CU_ASSERT(desc[0] != NULL); spdk_bdev_close(desc[0]); - /* Open bdev1 read/write. This should succeed. */ + /* + * Open bdev1 read/write. This should fail since bdev1 has been claimed + * by a vbdev module. + */ rc = spdk_bdev_open(bdev[1], true, NULL, NULL, &desc[1]); - CU_ASSERT(rc == 0); - CU_ASSERT(desc[1] != NULL); + CU_ASSERT(rc == -EPERM); /* - * Open bdev3 read/write. This should fail, since one of its - * base bdevs have been explicitly opened read/write. + * Open bdev3 read/write. This should fail since bdev3 has been claimed + * by a vbdev module. */ rc = spdk_bdev_open(bdev[3], true, NULL, NULL, &desc[3]); CU_ASSERT(rc == -EPERM); @@ -253,31 +268,17 @@ open_write_test(void) spdk_bdev_close(desc[3]); /* - * Open bdev7 read/write. This should fail, since one of its - * base bdevs have been explicitly opened read/write. This - * test ensures the bdev code traverses through multiple levels - * of base bdevs. + * Open bdev7 read/write. This should succeed since it is a leaf + * bdev. */ rc = spdk_bdev_open(bdev[7], true, NULL, NULL, &desc[7]); - CU_ASSERT(rc == -EPERM); - - /* Open bdev7 read-only. This should succeed. */ - rc = spdk_bdev_open(bdev[7], false, NULL, NULL, &desc[7]); CU_ASSERT(rc == 0); CU_ASSERT(desc[7] != NULL); spdk_bdev_close(desc[7]); - /* Reset tree by closing remaining descriptors. */ - spdk_bdev_close(desc[1]); - - /* Open bdev7 read/write. This should succeed. */ - rc = spdk_bdev_open(bdev[7], true, NULL, NULL, &desc[7]); - CU_ASSERT(rc == 0); - CU_ASSERT(desc[7] != NULL); - /* - * Open bdev4 read/write. This should fail, since one of its - * virtual bdevs has been explicitly opened read/write. + * Open bdev4 read/write. This should fail since bdev4 has been claimed + * by a vbdev module. */ rc = spdk_bdev_open(bdev[4], true, NULL, NULL, &desc[4]); CU_ASSERT(rc == -EPERM); @@ -288,24 +289,6 @@ open_write_test(void) CU_ASSERT(desc[4] != NULL); spdk_bdev_close(desc[4]); - /* - * Open bdev2 read/write. This should fail, since one of its - * virtual bdevs has been explicitly opened read/write. This - * test ensures the bdev code traverses through multiple levels - * of virtual bdevs. - */ - rc = spdk_bdev_open(bdev[2], true, NULL, NULL, &desc[2]); - CU_ASSERT(rc == -EPERM); - - /* Open bdev2 read-only. This should succeed. */ - rc = spdk_bdev_open(bdev[2], false, NULL, NULL, &desc[2]); - CU_ASSERT(rc == 0); - CU_ASSERT(desc[2] != NULL); - spdk_bdev_close(desc[2]); - - /* Reset tree by closing remaining descriptors. */ - spdk_bdev_close(desc[7]); - free_vbdev(bdev[7]); free_vbdev(bdev[3]);