From bd5a784719e62b9789cad5f10db8d114dcf04f62 Mon Sep 17 00:00:00 2001 From: Mike Gerdts Date: Tue, 20 Sep 2022 09:53:06 -0500 Subject: [PATCH] blob_bdev: support read-only devices External snapshots, which will be introduced in a later commit, will need read-only blob_bdev instances. This support is partially needed to support underlying devices that are naturally read-only and partially to provide an extra layer of protection against accidental writes. Signed-off-by: Mike Gerdts Change-Id: Ibcb28d00ad644a6053aa5f4de15471c2cd8e348a Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/14968 Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Shuhei Matsumoto Community-CI: Mellanox Build Bot Reviewed-by: Ben Walker --- include/spdk/blob_bdev.h | 29 ++++++++ module/blob/bdev/blob_bdev.c | 22 ++++-- test/unit/lib/blob/blob_bdev.c/blob_bdev_ut.c | 71 +++++++++++++++++++ 3 files changed, 118 insertions(+), 4 deletions(-) diff --git a/include/spdk/blob_bdev.h b/include/spdk/blob_bdev.h index 657ef065c..7193b10a1 100644 --- a/include/spdk/blob_bdev.h +++ b/include/spdk/blob_bdev.h @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: BSD-3-Clause * Copyright (C) 2017 Intel Corporation. * All rights reserved. + * Copyright (c) 2022 NVIDIA CORPORATION & AFFILIATES. All rights reserved. */ /** \file @@ -21,6 +22,17 @@ struct spdk_bs_dev; struct spdk_bdev; struct spdk_bdev_module; +struct spdk_bdev_bs_dev_opts { + /** + * The size of this structure according to the caller of this library is used for ABI + * compatibility. The library uses this field to know how many fields in this structure are + * valid. And the library will populate any remaining fields with default values. + * New added fields should be put at the end of the struct. + */ + size_t opts_size; +}; +SPDK_STATIC_ASSERT(sizeof(struct spdk_bdev_bs_dev_opts) == 8, "Incorrect size"); + /** * Create a blobstore block device from a bdev. * @@ -34,6 +46,23 @@ struct spdk_bdev_module; int spdk_bdev_create_bs_dev_ext(const char *bdev_name, spdk_bdev_event_cb_t event_cb, void *event_ctx, struct spdk_bs_dev **bs_dev); +/** + * Create a blobstore block device from a bdev. + * + * \param bdev_name The bdev to use. + * \param write If true, open device read-write, else open read-only. + * \param opts Additonal options; none currently supported. + * \param opts_size Size of structure referenced by opts. + * \param event_cb Called when the bdev triggers asynchronous event. + * \param event_ctx Argument passed to function event_cb. + * \param bs_dev Output parameter for a pointer to the blobstore block device. + * \return 0 if operation is successful, or suitable errno value otherwise. + */ +int spdk_bdev_create_bs_dev(const char *bdev_name, bool write, + struct spdk_bdev_bs_dev_opts *opts, size_t opts_size, + spdk_bdev_event_cb_t event_cb, void *event_ctx, + struct spdk_bs_dev **bs_dev); + /** * Claim the bdev module for the given blobstore. * diff --git a/module/blob/bdev/blob_bdev.c b/module/blob/bdev/blob_bdev.c index f074baef1..0a6689357 100644 --- a/module/blob/bdev/blob_bdev.c +++ b/module/blob/bdev/blob_bdev.c @@ -412,13 +412,20 @@ blob_bdev_init(struct blob_bdev *b, struct spdk_bdev_desc *desc) } int -spdk_bdev_create_bs_dev_ext(const char *bdev_name, spdk_bdev_event_cb_t event_cb, - void *event_ctx, struct spdk_bs_dev **_bs_dev) +spdk_bdev_create_bs_dev(const char *bdev_name, bool write, + struct spdk_bdev_bs_dev_opts *opts, size_t opts_size, + spdk_bdev_event_cb_t event_cb, void *event_ctx, + struct spdk_bs_dev **bs_dev) { struct blob_bdev *b; struct spdk_bdev_desc *desc; int rc; + if (opts != NULL && opts_size != sizeof(*opts)) { + SPDK_ERRLOG("bdev name '%s': unsupported options\n", bdev_name); + return -EINVAL; + } + b = calloc(1, sizeof(*b)); if (b == NULL) { @@ -426,7 +433,7 @@ spdk_bdev_create_bs_dev_ext(const char *bdev_name, spdk_bdev_event_cb_t event_cb return -ENOMEM; } - rc = spdk_bdev_open_ext(bdev_name, true, event_cb, event_ctx, &desc); + rc = spdk_bdev_open_ext(bdev_name, write, event_cb, event_ctx, &desc); if (rc != 0) { free(b); return rc; @@ -434,7 +441,14 @@ spdk_bdev_create_bs_dev_ext(const char *bdev_name, spdk_bdev_event_cb_t event_cb blob_bdev_init(b, desc); - *_bs_dev = &b->bs_dev; + *bs_dev = &b->bs_dev; return 0; } + +int +spdk_bdev_create_bs_dev_ext(const char *bdev_name, spdk_bdev_event_cb_t event_cb, + void *event_ctx, struct spdk_bs_dev **bs_dev) +{ + return spdk_bdev_create_bs_dev(bdev_name, true, NULL, 0, event_cb, event_ctx, bs_dev); +} diff --git a/test/unit/lib/blob/blob_bdev.c/blob_bdev_ut.c b/test/unit/lib/blob/blob_bdev.c/blob_bdev_ut.c index 46fd41bc3..2243bde4e 100644 --- a/test/unit/lib/blob/blob_bdev.c/blob_bdev_ut.c +++ b/test/unit/lib/blob/blob_bdev.c/blob_bdev_ut.c @@ -190,6 +190,75 @@ create_bs_dev(void) blob_bdev = (struct blob_bdev *)bs_dev; CU_ASSERT(blob_bdev->desc != NULL); + CU_ASSERT(blob_bdev->desc->write); + CU_ASSERT(blob_bdev->desc->bdev == g_bdev); + CU_ASSERT(blob_bdev->desc->claim_type == SPDK_BDEV_CLAIM_NONE); + CU_ASSERT(bdev.claim_type == SPDK_BDEV_CLAIM_NONE); + + bs_dev->destroy(bs_dev); + CU_ASSERT(bdev.open_cnt == 0); + g_bdev = NULL; +} + +static void +create_bs_dev_ro(void) +{ + struct spdk_bdev bdev; + struct spdk_bs_dev *bs_dev = NULL; + struct blob_bdev *blob_bdev; + struct spdk_bdev_bs_dev_opts opts = { 0 }; + int rc; + + /* opts with the wrong size returns -EINVAL */ + rc = spdk_bdev_create_bs_dev("nope", false, &opts, sizeof(opts) + 8, NULL, NULL, &bs_dev); + CU_ASSERT(rc == -EINVAL); + + /* opts with the right size is OK, but can still fail if the device doesn't exist. */ + opts.opts_size = sizeof(opts); + rc = spdk_bdev_create_bs_dev("nope", false, &opts, sizeof(opts), NULL, NULL, &bs_dev); + CU_ASSERT(rc == -ENODEV); + + init_bdev(&bdev, "bdev0", 16); + g_bdev = &bdev; + + /* The normal way to create a read-only device */ + rc = spdk_bdev_create_bs_dev("bdev0", false, NULL, 0, NULL, NULL, &bs_dev); + CU_ASSERT(rc == 0); + SPDK_CU_ASSERT_FATAL(bs_dev != NULL); + CU_ASSERT(bdev.open_cnt == 1); + + blob_bdev = (struct blob_bdev *)bs_dev; + CU_ASSERT(blob_bdev->desc != NULL); + CU_ASSERT(!blob_bdev->desc->write); + CU_ASSERT(blob_bdev->desc->bdev == g_bdev); + CU_ASSERT(blob_bdev->desc->claim_type == SPDK_BDEV_CLAIM_NONE); + CU_ASSERT(bdev.claim_type == SPDK_BDEV_CLAIM_NONE); + + bs_dev->destroy(bs_dev); + CU_ASSERT(bdev.open_cnt == 0); + g_bdev = NULL; +} + +static void +create_bs_dev_rw(void) +{ + struct spdk_bdev bdev; + struct spdk_bs_dev *bs_dev = NULL; + struct blob_bdev *blob_bdev; + int rc; + + init_bdev(&bdev, "bdev0", 16); + g_bdev = &bdev; + + /* This is equivalent to spdk_bdev_create_bs_dev_ext() */ + rc = spdk_bdev_create_bs_dev("bdev0", true, NULL, 0, NULL, NULL, &bs_dev); + CU_ASSERT(rc == 0); + SPDK_CU_ASSERT_FATAL(bs_dev != NULL); + CU_ASSERT(bdev.open_cnt == 1); + + blob_bdev = (struct blob_bdev *)bs_dev; + CU_ASSERT(blob_bdev->desc != NULL); + CU_ASSERT(blob_bdev->desc->write); CU_ASSERT(blob_bdev->desc->bdev == g_bdev); CU_ASSERT(blob_bdev->desc->claim_type == SPDK_BDEV_CLAIM_NONE); CU_ASSERT(bdev.claim_type == SPDK_BDEV_CLAIM_NONE); @@ -258,6 +327,8 @@ main(int argc, char **argv) suite = CU_add_suite("blob_bdev", NULL, NULL); CU_ADD_TEST(suite, create_bs_dev); + CU_ADD_TEST(suite, create_bs_dev_ro); + CU_ADD_TEST(suite, create_bs_dev_rw); CU_ADD_TEST(suite, claim_bs_dev); CU_basic_set_mode(CU_BRM_VERBOSE);