From fde584034d2974a58d0f88ae2a656c403d846854 Mon Sep 17 00:00:00 2001 From: Tomasz Zawadzki Date: Fri, 27 Oct 2017 15:47:34 +0200 Subject: [PATCH] lvol: give EEXIST error code on lvs name conflict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomasz Zawadzki Change-Id: I1df83cbd6414a1bb8f54328c735950b9476e323b Reviewed-on: https://review.gerrithub.io/384105 Reviewed-by: Jim Harris Reviewed-by: Piotr Pelpliński Tested-by: SPDK Automated Test System Reviewed-by: Daniel Verkamp --- include/spdk_internal/lvolstore.h | 1 + lib/bdev/lvol/vbdev_lvol.c | 8 +++++++- lib/lvol/lvol.c | 14 +++++++++----- test/unit/lib/lvol/lvol.c/lvol_ut.c | 6 +++--- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/include/spdk_internal/lvolstore.h b/include/spdk_internal/lvolstore.h index 5dbfdcf5f..d04700f20 100644 --- a/include/spdk_internal/lvolstore.h +++ b/include/spdk_internal/lvolstore.h @@ -60,6 +60,7 @@ struct spdk_lvs_with_handle_req { struct spdk_lvol_store *lvol_store; struct spdk_bs_dev *bs_dev; struct spdk_bdev *base_bdev; + int lvserrno; }; struct spdk_lvs_destroy_req { diff --git a/lib/bdev/lvol/vbdev_lvol.c b/lib/bdev/lvol/vbdev_lvol.c index 77dd64aef..0e5945e57 100644 --- a/lib/bdev/lvol/vbdev_lvol.c +++ b/lib/bdev/lvol/vbdev_lvol.c @@ -790,7 +790,13 @@ _vbdev_lvs_examine_cb(void *arg, struct spdk_lvol_store *lvol_store, int lvserrn struct spdk_lvs_with_handle_req *req = (struct spdk_lvs_with_handle_req *)arg; struct spdk_lvol *lvol, *tmp; - if (lvserrno != 0) { + if (lvserrno == -EEXIST) { + SPDK_INFOLOG(SPDK_TRACE_VBDEV_LVOL, + "Name for lvolstore on device %s conflicts with name for already loaded lvs\n", + req->base_bdev->name); + spdk_bdev_module_examine_done(SPDK_GET_BDEV_MODULE(lvol)); + goto end; + } else if (lvserrno != 0) { SPDK_INFOLOG(SPDK_TRACE_VBDEV_LVOL, "Lvol store not found on %s\n", req->base_bdev->name); spdk_bdev_module_examine_done(SPDK_GET_BDEV_MODULE(lvol)); goto end; diff --git a/lib/lvol/lvol.c b/lib/lvol/lvol.c index d08bcae77..50c32adcd 100644 --- a/lib/lvol/lvol.c +++ b/lib/lvol/lvol.c @@ -211,7 +211,7 @@ _spdk_bs_unload_with_error_cb(void *cb_arg, int lvolerrno) { struct spdk_lvs_with_handle_req *req = (struct spdk_lvs_with_handle_req *)cb_arg; - req->cb_fn(req->cb_arg, NULL, -ENODEV); + req->cb_fn(req->cb_arg, NULL, req->lvserrno); free(req); } @@ -225,6 +225,7 @@ _spdk_close_super_cb(void *cb_arg, int lvolerrno) if (lvolerrno != 0) { SPDK_INFOLOG(SPDK_TRACE_LVOL, "Could not close super blob\n"); _spdk_lvs_free(lvs); + req->lvserrno = -ENODEV; spdk_bs_unload(bs, _spdk_bs_unload_with_error_cb, req); return; } @@ -258,6 +259,7 @@ _spdk_lvs_read_uuid(void *cb_arg, struct spdk_blob *blob, int lvolerrno) if (lvolerrno != 0) { SPDK_INFOLOG(SPDK_TRACE_LVOL, "Could not open super blob\n"); _spdk_lvs_free(lvs); + req->lvserrno = -ENODEV; spdk_bs_unload(bs, _spdk_bs_unload_with_error_cb, req); return; } @@ -265,12 +267,14 @@ _spdk_lvs_read_uuid(void *cb_arg, struct spdk_blob *blob, int lvolerrno) rc = spdk_bs_md_get_xattr_value(blob, "uuid", (const void **)&attr, &value_len); if (rc != 0 || value_len != UUID_STRING_LEN || attr[UUID_STRING_LEN - 1] != '\0') { SPDK_INFOLOG(SPDK_TRACE_LVOL, "missing or incorrect UUID\n"); + req->lvserrno = -EINVAL; spdk_bs_md_close_blob(&blob, _spdk_close_super_blob_with_error_cb, req); return; } if (uuid_parse(attr, lvs->uuid)) { SPDK_INFOLOG(SPDK_TRACE_LVOL, "incorrect UUID '%s'\n", attr); + req->lvserrno = -EINVAL; spdk_bs_md_close_blob(&blob, _spdk_close_super_blob_with_error_cb, req); return; } @@ -278,6 +282,7 @@ _spdk_lvs_read_uuid(void *cb_arg, struct spdk_blob *blob, int lvolerrno) rc = spdk_bs_md_get_xattr_value(blob, "name", (const void **)&attr, &value_len); if (rc != 0 || value_len > SPDK_LVS_NAME_MAX) { SPDK_INFOLOG(SPDK_TRACE_LVOL, "missing or invalid name\n"); + req->lvserrno = -EINVAL; spdk_bs_md_close_blob(&blob, _spdk_close_super_blob_with_error_cb, req); return; } @@ -287,6 +292,7 @@ _spdk_lvs_read_uuid(void *cb_arg, struct spdk_blob *blob, int lvolerrno) rc = _spdk_add_lvs_to_list(lvs); if (rc) { SPDK_INFOLOG(SPDK_TRACE_LVOL, "lvolstore with name %s already exists\n", lvs->name); + req->lvserrno = -EEXIST; spdk_bs_md_close_blob(&blob, _spdk_close_super_blob_with_error_cb, req); return; } @@ -306,6 +312,7 @@ _spdk_lvs_open_super(void *cb_arg, spdk_blob_id blobid, int lvolerrno) if (lvolerrno != 0) { SPDK_INFOLOG(SPDK_TRACE_LVOL, "Super blob not found\n"); _spdk_lvs_free(lvs); + req->lvserrno = -ENODEV; spdk_bs_unload(bs, _spdk_bs_unload_with_error_cb, req); return; } @@ -554,15 +561,12 @@ spdk_lvs_init(struct spdk_bs_dev *bs_dev, struct spdk_lvs_opts *o, strncpy(lvs->name, o->name, SPDK_LVS_NAME_MAX); rc = _spdk_add_lvs_to_list(lvs); - if (rc) { SPDK_ERRLOG("lvolstore with name %s already exists\n", lvs->name); _spdk_lvs_free(lvs); - return -EINVAL; + return -EEXIST; } - _spdk_add_lvs_to_list(lvs); - lvs_req = calloc(1, sizeof(*lvs_req)); if (!lvs_req) { _spdk_lvs_free(lvs); diff --git a/test/unit/lib/lvol/lvol.c/lvol_ut.c b/test/unit/lib/lvol/lvol.c/lvol_ut.c index d02f586f4..703625bb4 100644 --- a/test/unit/lib/lvol/lvol.c/lvol_ut.c +++ b/test/unit/lib/lvol/lvol.c/lvol_ut.c @@ -598,7 +598,7 @@ lvs_names(void) /* Test that we cannot create another lvolstore with name 'x'. */ rc = spdk_lvs_init(&dev_x2.bs_dev, &opts_x, lvol_store_op_with_handle_complete, NULL); - CU_ASSERT(rc == -EINVAL); + CU_ASSERT(rc == -EEXIST); /* Now destroy lvolstore 'x' and then confirm we can create a new lvolstore with name 'x'. */ g_lvserrno = -1; @@ -983,7 +983,7 @@ lvs_load(void) g_lvserrno = 0; super_blob->open_status = 0; spdk_lvs_load(&dev.bs_dev, lvol_store_op_with_handle_complete, req); - CU_ASSERT(g_lvserrno == -ENODEV); + CU_ASSERT(g_lvserrno == -EINVAL); CU_ASSERT(g_lvol_store == NULL); CU_ASSERT(TAILQ_EMPTY(&g_lvol_stores)); @@ -991,7 +991,7 @@ lvs_load(void) g_lvserrno = 0; spdk_blob_md_set_xattr(super_blob, "uuid", uuid, UUID_STRING_LEN); spdk_lvs_load(&dev.bs_dev, lvol_store_op_with_handle_complete, req); - CU_ASSERT(g_lvserrno == -ENODEV); + CU_ASSERT(g_lvserrno == -EINVAL); CU_ASSERT(g_lvol_store == NULL); CU_ASSERT(TAILQ_EMPTY(&g_lvol_stores));