bdev/ocssd: Remove range parameter from bdev_ocssd_create RPC

It has been confirmed that there is no affected use case in
the SPDK community when we remove the range parameter from
the bdev_ocssd_create RPC.

Hence, remove the range parameter from the bdev_ocssd_create RPC,
remove range parameter from bdev_ocssd_create_bdev(), remove range
info from ocssd_bdev_config_json(), and then update unit tests
accordingly.

Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I1b0a541b61bf26732fd028dc43becb7ca2384f8e
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6220
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Konrad Sztyber <konrad.sztyber@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Community-CI: Broadcom CI
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
This commit is contained in:
Shuhei Matsumoto 2021-02-15 06:03:06 +09:00 committed by Tomasz Zawadzki
parent bd8de45801
commit 20f1cf632b
7 changed files with 19 additions and 252 deletions

View File

@ -2,6 +2,11 @@
## v21.04: (Upcoming Release)
### bdev
For `bdev_ocssd_create` RPC, the optional parameter `range` was removed.
Only one OCSSD bdev can be created for one OCSSD namespace.
### env
Added spdk_pci_device_allow API to allow applications to add PCI addresses to

View File

@ -127,21 +127,10 @@ ocssd_bdev_config_json(struct spdk_json_write_ctx *w, struct nvme_bdev *nvme_bde
{
struct nvme_bdev_ns *nvme_ns;
struct nvme_bdev_ctrlr *nvme_bdev_ctrlr;
struct ocssd_bdev *ocssd_bdev;
char range_buf[128];
int rc;
nvme_ns = nvme_bdev_to_bdev_ns(nvme_bdev);
assert(nvme_ns != NULL);
nvme_bdev_ctrlr = nvme_ns->ctrlr;
ocssd_bdev = SPDK_CONTAINEROF(nvme_bdev, struct ocssd_bdev, nvme_bdev);
rc = snprintf(range_buf, sizeof(range_buf), "%"PRIu64"-%"PRIu64,
ocssd_bdev->range.begin, ocssd_bdev->range.end);
if (rc < 0 || rc >= (int)sizeof(range_buf)) {
SPDK_ERRLOG("Failed to convert parallel unit range\n");
return;
}
spdk_json_write_object_begin(w);
spdk_json_write_named_string(w, "method", "bdev_ocssd_create");
@ -150,7 +139,6 @@ ocssd_bdev_config_json(struct spdk_json_write_ctx *w, struct nvme_bdev *nvme_bde
spdk_json_write_named_string(w, "ctrlr_name", nvme_bdev_ctrlr->name);
spdk_json_write_named_string(w, "bdev_name", nvme_bdev->disk.name);
spdk_json_write_named_uint32(w, "nsid", nvme_ns->id);
spdk_json_write_named_string(w, "range", range_buf);
spdk_json_write_object_end(w);
spdk_json_write_object_end(w);
@ -1263,43 +1251,9 @@ bdev_ocssd_init_zones(struct bdev_ocssd_create_ctx *create_ctx)
return bdev_ocssd_init_zone(create_ctx);
}
static bool
bdev_ocssd_verify_range(struct nvme_bdev_ns *nvme_ns,
const struct bdev_ocssd_range *range)
{
struct bdev_ocssd_ns *ocssd_ns = bdev_ocssd_get_ns_from_nvme(nvme_ns);
const struct spdk_ocssd_geometry_data *geometry = &ocssd_ns->geometry;
struct ocssd_bdev *ocssd_bdev;
struct nvme_bdev *nvme_bdev;
size_t num_punits = geometry->num_pu * geometry->num_grp;
/* First verify the range is within the geometry */
if (range != NULL && (range->begin > range->end || range->end >= num_punits)) {
return false;
}
TAILQ_FOREACH(nvme_bdev, &nvme_ns->bdevs, tailq) {
ocssd_bdev = SPDK_CONTAINEROF(nvme_bdev, struct ocssd_bdev, nvme_bdev);
/* Empty range means whole namespace should be used */
if (range == NULL) {
return false;
}
/* Make sure the range doesn't overlap with any other range */
if (range->begin <= ocssd_bdev->range.end &&
range->end >= ocssd_bdev->range.begin) {
return false;
}
}
return true;
}
void
bdev_ocssd_create_bdev(const char *ctrlr_name, const char *bdev_name, uint32_t nsid,
const struct bdev_ocssd_range *range, bdev_ocssd_create_cb cb_fn,
void *cb_arg)
bdev_ocssd_create_cb cb_fn, void *cb_arg)
{
struct nvme_bdev_ctrlr *nvme_bdev_ctrlr;
struct bdev_ocssd_create_ctx *create_ctx = NULL;
@ -1352,12 +1306,6 @@ bdev_ocssd_create_bdev(const char *ctrlr_name, const char *bdev_name, uint32_t n
goto error;
}
if (!bdev_ocssd_verify_range(nvme_ns, range)) {
SPDK_ERRLOG("Invalid parallel unit range\n");
rc = -EINVAL;
goto error;
}
ocssd_bdev = calloc(1, sizeof(*ocssd_bdev));
if (!ocssd_bdev) {
rc = -ENOMEM;
@ -1374,18 +1322,14 @@ bdev_ocssd_create_bdev(const char *ctrlr_name, const char *bdev_name, uint32_t n
create_ctx->nvme_ns = nvme_ns;
create_ctx->cb_fn = cb_fn;
create_ctx->cb_arg = cb_arg;
create_ctx->range = range;
create_ctx->range = NULL;
nvme_bdev = &ocssd_bdev->nvme_bdev;
nvme_bdev->nvme_ns = nvme_ns;
geometry = &ocssd_ns->geometry;
if (range != NULL) {
ocssd_bdev->range = *range;
} else {
ocssd_bdev->range.begin = 0;
ocssd_bdev->range.end = geometry->num_grp * geometry->num_pu - 1;
}
nvme_bdev->disk.name = strdup(bdev_name);
if (!nvme_bdev->disk.name) {

View File

@ -46,7 +46,6 @@ typedef void (*bdev_ocssd_create_cb)(const char *bdev_name, int status, void *ct
typedef void (*bdev_ocssd_delete_cb)(int status, void *ctx);
void bdev_ocssd_create_bdev(const char *ctrlr_name, const char *bdev_name, uint32_t nsid,
const struct bdev_ocssd_range *range,
bdev_ocssd_create_cb cb_fn, void *cb_arg);
void bdev_ocssd_delete_bdev(const char *bdev_name, bdev_ocssd_delete_cb cb_fn, void *cb_arg);

View File

@ -45,14 +45,12 @@ struct rpc_create_ocssd_bdev {
char *ctrlr_name;
char *bdev_name;
uint32_t nsid;
char *range;
};
static const struct spdk_json_object_decoder rpc_create_ocssd_bdev_decoders[] = {
{"ctrlr_name", offsetof(struct rpc_create_ocssd_bdev, ctrlr_name), spdk_json_decode_string},
{"bdev_name", offsetof(struct rpc_create_ocssd_bdev, bdev_name), spdk_json_decode_string},
{"nsid", offsetof(struct rpc_create_ocssd_bdev, nsid), spdk_json_decode_uint32, true},
{"range", offsetof(struct rpc_create_ocssd_bdev, range), spdk_json_decode_string, true},
};
static void
@ -60,13 +58,11 @@ free_rpc_create_ocssd_bdev(struct rpc_create_ocssd_bdev *rpc)
{
free(rpc->ctrlr_name);
free(rpc->bdev_name);
free(rpc->range);
}
struct rpc_bdev_ocssd_create_ctx {
struct spdk_jsonrpc_request *request;
struct rpc_create_ocssd_bdev rpc;
struct bdev_ocssd_range range;
};
static void
@ -92,8 +88,6 @@ static void
rpc_bdev_ocssd_create(struct spdk_jsonrpc_request *request, const struct spdk_json_val *params)
{
struct rpc_bdev_ocssd_create_ctx *ctx;
struct bdev_ocssd_range *range = NULL;
int rc;
ctx = calloc(1, sizeof(*ctx));
if (!ctx) {
@ -111,19 +105,8 @@ rpc_bdev_ocssd_create(struct spdk_jsonrpc_request *request, const struct spdk_js
goto out;
}
if (ctx->rpc.range != NULL) {
rc = sscanf(ctx->rpc.range, "%"PRIu64"-%"PRIu64,
&ctx->range.begin, &ctx->range.end);
if (rc != 2) {
spdk_jsonrpc_send_error_response(request, -EINVAL, "Failed to parse range");
goto out;
}
range = &ctx->range;
}
bdev_ocssd_create_bdev(ctx->rpc.ctrlr_name, ctx->rpc.bdev_name, ctx->rpc.nsid,
range, rpc_bdev_ocssd_create_done, ctx);
rpc_bdev_ocssd_create_done, ctx);
return;
out:
free_rpc_create_ocssd_bdev(&ctx->rpc);

View File

@ -2305,16 +2305,13 @@ Format: 'user:u1 secret:s1 muser:mu1 msecret:ms1,user:u2 secret:s2 muser:mu2 mse
print_json(rpc.bdev.bdev_ocssd_create(args.client,
ctrlr_name=args.ctrlr_name,
bdev_name=args.name,
nsid=nsid,
range=args.range))
nsid=nsid))
p = subparsers.add_parser('bdev_ocssd_create',
help='Creates zoned bdev on specified Open Channel controller')
p.add_argument('-c', '--ctrlr_name', help='Name of the OC NVMe controller', required=True)
p.add_argument('-b', '--name', help='Name of the bdev to create', required=True)
p.add_argument('-n', '--nsid', help='Namespace ID', required=False)
p.add_argument('-r', '--range', help='Parallel unit range (in the form of BEGIN-END (inclusive))',
required=False)
p.set_defaults(func=bdev_ocssd_create)
def bdev_ocssd_delete(args):

View File

@ -1032,7 +1032,6 @@ def bdev_ocssd_create(client, ctrlr_name, bdev_name, nsid=None, range=None):
ctrlr_name: name of the OC NVMe controller
bdev_name: name of the bdev to create
nsid: namespace ID
range: parallel unit range
"""
params = {'ctrlr_name': ctrlr_name,
'bdev_name': bdev_name}
@ -1040,9 +1039,6 @@ def bdev_ocssd_create(client, ctrlr_name, bdev_name, nsid=None, range=None):
if nsid is not None:
params['nsid'] = nsid
if range is not None:
params['range'] = range
return client.call('bdev_ocssd_create', params)

View File

@ -512,12 +512,11 @@ create_bdev_cb(const char *bdev_name, int status, void *ctx)
}
static int
create_bdev(const char *ctrlr_name, const char *bdev_name, uint32_t nsid,
const struct bdev_ocssd_range *range)
create_bdev(const char *ctrlr_name, const char *bdev_name, uint32_t nsid)
{
int status = EFAULT;
bdev_ocssd_create_bdev(ctrlr_name, bdev_name, nsid, range, create_bdev_cb, &status);
bdev_ocssd_create_bdev(ctrlr_name, bdev_name, nsid, create_bdev_cb, &status);
while (spdk_thread_poll(g_thread, 0, 0) > 0) {}
@ -551,7 +550,6 @@ test_create_controller(void)
struct spdk_nvme_transport_id trid = { .traddr = "00:00:00" };
struct spdk_ocssd_geometry_data geometry = {};
struct spdk_bdev *bdev;
struct bdev_ocssd_range range;
const char *controller_name = "nvme0";
const size_t ns_count = 16;
char namebuf[128];
@ -579,7 +577,7 @@ test_create_controller(void)
for (nsid = 1; nsid <= ns_count; ++nsid) {
snprintf(namebuf, sizeof(namebuf), "%sn%"PRIu32, controller_name, nsid);
rc = create_bdev(controller_name, namebuf, nsid, NULL);
rc = create_bdev(controller_name, namebuf, nsid);
CU_ASSERT_EQUAL(rc, 0);
bdev = spdk_bdev_get_by_name(namebuf);
@ -594,7 +592,7 @@ test_create_controller(void)
for (nsid = 1; nsid <= ns_count; ++nsid) {
snprintf(namebuf, sizeof(namebuf), "%sn%"PRIu32, controller_name, nsid);
rc = create_bdev(controller_name, namebuf, nsid, NULL);
rc = create_bdev(controller_name, namebuf, nsid);
CU_ASSERT_EQUAL(rc, 0);
bdev = spdk_bdev_get_by_name(namebuf);
@ -604,46 +602,6 @@ test_create_controller(void)
delete_nvme_bdev_controller(nvme_bdev_ctrlr);
nvme_bdev_ctrlr = create_nvme_bdev_controller(&trid, controller_name);
/* Verify it's not possible to create a bdev on non-existent namespace */
rc = create_bdev(controller_name, "invalid", ns_count + 1, NULL);
CU_ASSERT_EQUAL(rc, -ENODEV);
delete_nvme_bdev_controller(nvme_bdev_ctrlr);
/* Verify the correctness of parallel unit range validation */
nvme_bdev_ctrlr = create_nvme_bdev_controller(&trid, controller_name);
range.begin = 0;
range.end = geometry.num_grp * geometry.num_pu;
rc = create_bdev(controller_name, "invalid", 1, &range);
CU_ASSERT_EQUAL(rc, -EINVAL);
/* Verify it's not possible for the bdevs to overlap */
range.begin = 0;
range.end = 16;
rc = create_bdev(controller_name, "valid", 1, &range);
CU_ASSERT_EQUAL(rc, 0);
bdev = spdk_bdev_get_by_name("valid");
CU_ASSERT_PTR_NOT_NULL(bdev);
range.begin = 16;
range.end = 31;
rc = create_bdev(controller_name, "invalid", 1, &range);
CU_ASSERT_EQUAL(rc, -EINVAL);
/* But it is possible to create them without overlap */
range.begin = 17;
range.end = 31;
rc = create_bdev(controller_name, "valid2", 1, &range);
CU_ASSERT_EQUAL(rc, 0);
bdev = spdk_bdev_get_by_name("valid2");
CU_ASSERT_PTR_NOT_NULL(bdev);
delete_nvme_bdev_controller(nvme_bdev_ctrlr);
free_controller(ctrlr);
}
@ -678,7 +636,7 @@ test_device_geometry(void)
ctrlr = create_controller(&trid, 1, &geometry);
nvme_bdev_ctrlr = create_nvme_bdev_controller(&trid, controller_name);
rc = create_bdev(controller_name, bdev_name, 1, NULL);
rc = create_bdev(controller_name, bdev_name, 1);
CU_ASSERT_EQUAL(rc, 0);
bdev = spdk_bdev_get_by_name(bdev_name);
@ -754,7 +712,7 @@ test_lba_translation(void)
SPDK_CU_ASSERT_FATAL(nvme_bdev_ctrlr->namespaces[0] != NULL);
ocssd_ns = bdev_ocssd_get_ns_from_nvme(nvme_bdev_ctrlr->namespaces[0]);
rc = create_bdev(controller_name, bdev_name, 1, NULL);
rc = create_bdev(controller_name, bdev_name, 1);
CU_ASSERT_EQUAL(rc, 0);
bdev = spdk_bdev_get_by_name(bdev_name);
@ -810,7 +768,7 @@ test_lba_translation(void)
SPDK_CU_ASSERT_FATAL(nvme_bdev_ctrlr->namespaces[0] != NULL);
ocssd_ns = bdev_ocssd_get_ns_from_nvme(nvme_bdev_ctrlr->namespaces[0]);
rc = create_bdev(controller_name, bdev_name, 1, NULL);
rc = create_bdev(controller_name, bdev_name, 1);
CU_ASSERT_EQUAL(rc, 0);
bdev = spdk_bdev_get_by_name(bdev_name);
@ -851,120 +809,6 @@ test_lba_translation(void)
free_controller(ctrlr);
}
static void
punit_range_to_addr(const struct spdk_nvme_ctrlr *ctrlr, uint64_t punit,
uint64_t *grp, uint64_t *pu)
{
const struct spdk_ocssd_geometry_data *geo = &ctrlr->geometry;
*grp = punit / geo->num_pu;
*pu = punit % geo->num_pu;
CU_ASSERT(*grp < geo->num_grp);
}
static void
test_parallel_unit_range(void)
{
struct spdk_nvme_ctrlr *ctrlr;
struct nvme_bdev_ctrlr *nvme_bdev_ctrlr;
struct spdk_nvme_transport_id trid = { .traddr = "00:00:00" };
const char *controller_name = "nvme0";
const char *bdev_name[] = { "nvme0n1", "nvme0n2", "nvme0n3" };
const struct bdev_ocssd_range range[3] = { { 0, 5 }, { 6, 18 }, { 19, 23 } };
struct bdev_ocssd_ns *ocssd_ns;
struct ocssd_bdev *ocssd_bdev[3];
struct spdk_ocssd_geometry_data geometry = {};
struct spdk_bdev *bdev[3];
uint64_t lba, i, offset, grp, pu, zone_size;
int rc;
geometry = (struct spdk_ocssd_geometry_data) {
.clba = 500,
.num_chk = 60,
.num_pu = 8,
.num_grp = 3,
.lbaf = {
.lbk_len = 9,
.chk_len = 6,
.pu_len = 3,
.grp_len = 2,
}
};
ctrlr = create_controller(&trid, 1, &geometry);
nvme_bdev_ctrlr = create_nvme_bdev_controller(&trid, controller_name);
SPDK_CU_ASSERT_FATAL(nvme_bdev_ctrlr != NULL);
SPDK_CU_ASSERT_FATAL(nvme_bdev_ctrlr->namespaces[0] != NULL);
ocssd_ns = bdev_ocssd_get_ns_from_nvme(nvme_bdev_ctrlr->namespaces[0]);
for (i = 0; i < SPDK_COUNTOF(range); ++i) {
rc = create_bdev(controller_name, bdev_name[i], 1, &range[i]);
CU_ASSERT_EQUAL(rc, 0);
bdev[i] = spdk_bdev_get_by_name(bdev_name[i]);
SPDK_CU_ASSERT_FATAL(bdev[i] != NULL);
ocssd_bdev[i] = SPDK_CONTAINEROF(bdev[i], struct ocssd_bdev, nvme_bdev.disk);
}
zone_size = bdev[0]->zone_size;
CU_ASSERT_EQUAL(zone_size, bdev[1]->zone_size);
CU_ASSERT_EQUAL(zone_size, bdev[2]->zone_size);
/* Verify the first addresses are correct */
lba = bdev_ocssd_to_disk_lba(ocssd_bdev[0], ocssd_ns, 0);
CU_ASSERT_EQUAL(lba, generate_lba(&geometry, 0, 0, 0, 0));
CU_ASSERT_EQUAL(bdev_ocssd_from_disk_lba(ocssd_bdev[0], ocssd_ns, lba), 0);
lba = bdev_ocssd_to_disk_lba(ocssd_bdev[1], ocssd_ns, 0);
CU_ASSERT_EQUAL(lba, generate_lba(&geometry, 0, 0, 6, 0));
CU_ASSERT_EQUAL(bdev_ocssd_from_disk_lba(ocssd_bdev[1], ocssd_ns, lba), 0);
lba = bdev_ocssd_to_disk_lba(ocssd_bdev[2], ocssd_ns, 0);
CU_ASSERT_EQUAL(lba, generate_lba(&geometry, 0, 0, 3, 2));
CU_ASSERT_EQUAL(bdev_ocssd_from_disk_lba(ocssd_bdev[2], ocssd_ns, lba), 0);
/* Verify last address correctness */
lba = bdev_ocssd_to_disk_lba(ocssd_bdev[0], ocssd_ns, bdev[0]->blockcnt - 1);
CU_ASSERT_EQUAL(lba, generate_lba(&geometry, geometry.clba - 1, geometry.num_chk - 1, 5, 0));
CU_ASSERT_EQUAL(bdev_ocssd_from_disk_lba(ocssd_bdev[0], ocssd_ns, lba), bdev[0]->blockcnt - 1);
lba = bdev_ocssd_to_disk_lba(ocssd_bdev[1], ocssd_ns, bdev[1]->blockcnt - 1);
CU_ASSERT_EQUAL(lba, generate_lba(&geometry, geometry.clba - 1, geometry.num_chk - 1, 2, 2));
CU_ASSERT_EQUAL(bdev_ocssd_from_disk_lba(ocssd_bdev[1], ocssd_ns, lba), bdev[1]->blockcnt - 1);
lba = bdev_ocssd_to_disk_lba(ocssd_bdev[2], ocssd_ns, bdev[2]->blockcnt - 1);
CU_ASSERT_EQUAL(lba, generate_lba(&geometry, geometry.clba - 1, geometry.num_chk - 1, 7, 2));
CU_ASSERT_EQUAL(bdev_ocssd_from_disk_lba(ocssd_bdev[2], ocssd_ns, lba), bdev[2]->blockcnt - 1);
/* Verify correct jumps across parallel units / groups */
for (i = 0; i < SPDK_COUNTOF(range); ++i) {
for (offset = 0; offset < bdev_ocssd_num_parallel_units(ocssd_bdev[i]); ++offset) {
punit_range_to_addr(ctrlr, range[i].begin + offset, &grp, &pu);
lba = bdev_ocssd_to_disk_lba(ocssd_bdev[i], ocssd_ns, offset * zone_size + 68);
CU_ASSERT_EQUAL(lba, generate_lba(&geometry, 68, 0, pu, grp));
CU_ASSERT_EQUAL(bdev_ocssd_from_disk_lba(ocssd_bdev[i], ocssd_ns, lba),
offset * zone_size + 68);
}
}
/* Verify correct address wrapping */
for (i = 0; i < SPDK_COUNTOF(range); ++i) {
punit_range_to_addr(ctrlr, range[i].begin, &grp, &pu);
offset = bdev_ocssd_num_parallel_units(ocssd_bdev[i]) * zone_size + 68;
lba = bdev_ocssd_to_disk_lba(ocssd_bdev[i], ocssd_ns, offset);
CU_ASSERT_EQUAL(lba, generate_lba(&geometry, 68, 1, pu, grp));
assert(lba == generate_lba(&geometry, 68, 1, pu, grp));
CU_ASSERT_EQUAL(bdev_ocssd_from_disk_lba(ocssd_bdev[i], ocssd_ns, lba), offset);
}
delete_nvme_bdev_controller(nvme_bdev_ctrlr);
free_controller(ctrlr);
}
static void
get_zone_info_cb(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg)
{
@ -1062,7 +906,7 @@ test_get_zone_info(void)
ctrlr = create_controller(&trid, 1, &geometry);
nvme_bdev_ctrlr = create_nvme_bdev_controller(&trid, controller_name);
rc = create_bdev(controller_name, bdev_name, 1, NULL);
rc = create_bdev(controller_name, bdev_name, 1);
CU_ASSERT_EQUAL(rc, 0);
bdev = spdk_bdev_get_by_name(bdev_name);
@ -1207,7 +1051,6 @@ main(int argc, const char **argv)
CU_ADD_TEST(suite, test_create_controller);
CU_ADD_TEST(suite, test_device_geometry);
CU_ADD_TEST(suite, test_lba_translation);
CU_ADD_TEST(suite, test_parallel_unit_range);
CU_ADD_TEST(suite, test_get_zone_info);
g_thread = spdk_thread_create("test", NULL);