diff --git a/module/bdev/lvol/vbdev_lvol.c b/module/bdev/lvol/vbdev_lvol.c index 785fdcd08..fd6c607db 100644 --- a/module/bdev/lvol/vbdev_lvol.c +++ b/module/bdev/lvol/vbdev_lvol.c @@ -639,6 +639,11 @@ vbdev_lvol_destroy(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb ctx->cb_fn = cb_fn; ctx->cb_arg = cb_arg; + if (spdk_blob_is_degraded(lvol->blob)) { + spdk_lvol_close(lvol, _vbdev_lvol_destroy_cb, ctx); + return; + } + spdk_bdev_unregister(lvol->bdev, _vbdev_lvol_destroy_cb, ctx); } diff --git a/module/bdev/lvol/vbdev_lvol_rpc.c b/module/bdev/lvol/vbdev_lvol_rpc.c index ec64ef85e..dc4c00faa 100644 --- a/module/bdev/lvol/vbdev_lvol_rpc.c +++ b/module/bdev/lvol/vbdev_lvol_rpc.c @@ -1016,6 +1016,8 @@ rpc_bdev_lvol_delete(struct spdk_jsonrpc_request *request, struct rpc_bdev_lvol_delete req = {}; struct spdk_bdev *bdev; struct spdk_lvol *lvol; + struct spdk_uuid uuid; + char *lvs_name, *lvol_name; if (spdk_json_decode_object(params, rpc_bdev_lvol_delete_decoders, SPDK_COUNTOF(rpc_bdev_lvol_delete_decoders), @@ -1026,19 +1028,40 @@ rpc_bdev_lvol_delete(struct spdk_jsonrpc_request *request, goto cleanup; } + /* lvol is not degraded, get lvol via bdev name or alias */ bdev = spdk_bdev_get_by_name(req.name); - if (bdev == NULL) { - SPDK_ERRLOG("no bdev for provided name %s\n", req.name); - spdk_jsonrpc_send_error_response(request, -ENODEV, spdk_strerror(ENODEV)); - goto cleanup; + if (bdev != NULL) { + lvol = vbdev_lvol_get_from_bdev(bdev); + if (lvol != NULL) { + goto done; + } } - lvol = vbdev_lvol_get_from_bdev(bdev); - if (lvol == NULL) { - spdk_jsonrpc_send_error_response(request, -ENODEV, spdk_strerror(ENODEV)); - goto cleanup; + /* lvol is degraded, get lvol via UUID */ + if (spdk_uuid_parse(&uuid, req.name) == 0) { + lvol = spdk_lvol_get_by_uuid(&uuid); + if (lvol != NULL) { + goto done; + } } + /* lvol is degraded, get lvol via lvs_name/lvol_name */ + lvol_name = strchr(req.name, '/'); + if (lvol_name != NULL) { + *lvol_name = '\0'; + lvol_name++; + lvs_name = req.name; + lvol = spdk_lvol_get_by_names(lvs_name, lvol_name); + if (lvol != NULL) { + goto done; + } + } + + /* Could not find lvol, degraded or not. */ + spdk_jsonrpc_send_error_response(request, -ENODEV, spdk_strerror(ENODEV)); + goto cleanup; + +done: vbdev_lvol_destroy(lvol, rpc_bdev_lvol_delete_cb, request); cleanup: diff --git a/test/lvol/esnap/esnap.c b/test/lvol/esnap/esnap.c index 27ff79676..6c30224f4 100644 --- a/test/lvol/esnap/esnap.c +++ b/test/lvol/esnap/esnap.c @@ -116,6 +116,14 @@ lvol_op_with_handle_cb(void *cb_arg, struct spdk_lvol *lvol, int lvserrno) data->lvserrno = lvserrno; } +static void +lvol_op_complete_cb(void *cb_arg, int lvolerrno) +{ + int *err = cb_arg; + + *err = lvolerrno; +} + static void ut_event_cb(enum spdk_bdev_event_type type, struct spdk_bdev *bdev, void *event_ctx) { @@ -445,6 +453,224 @@ esnap_hotplug(void) CU_ASSERT(rc == 0); } +static void +esnap_remove_degraded(void) +{ + const char *uuid_esnap = "33358eb9-3dcf-4275-b089-0becc126fc3d"; + struct malloc_bdev_opts malloc_opts = { 0 }; + const uint32_t bs_size_bytes = 10 * 1024 * 1024; + const uint32_t bs_block_size = 4096; + const uint32_t cluster_size = 32 * 1024; + const uint32_t esnap_size_bytes = 2 * cluster_size; + struct op_with_handle_data owh_data = { 0 }; + struct spdk_lvol_store *lvs; + struct spdk_bdev *malloc_bdev = NULL; + struct spdk_lvol *vol1, *vol2, *vol3; + char aiopath[PATH_MAX]; + int rc, rc2; + + g_bdev_opts.bdev_auto_examine = true; + + /* Create aio device to hold the lvstore. */ + rc = make_test_file(bs_size_bytes, aiopath, sizeof(aiopath), "remove_degraded.aio"); + SPDK_CU_ASSERT_FATAL(rc == 0); + rc = create_aio_bdev("aio1", aiopath, bs_block_size, false); + SPDK_CU_ASSERT_FATAL(rc == 0); + poll_threads(); + + rc = vbdev_lvs_create("aio1", "lvs1", cluster_size, 0, 0, + lvs_op_with_handle_cb, clear_owh(&owh_data)); + SPDK_CU_ASSERT_FATAL(rc == 0); + poll_error_updated(&owh_data.lvserrno); + SPDK_CU_ASSERT_FATAL(owh_data.lvserrno == 0); + SPDK_CU_ASSERT_FATAL(owh_data.u.lvs != NULL); + lvs = owh_data.u.lvs; + + /* Create esnap device */ + spdk_uuid_parse(&malloc_opts.uuid, uuid_esnap); + malloc_opts.name = "esnap"; + malloc_opts.num_blocks = esnap_size_bytes / bs_block_size; + malloc_opts.block_size = bs_block_size; + rc = create_malloc_disk(&malloc_bdev, &malloc_opts); + SPDK_CU_ASSERT_FATAL(rc == 0); + + /* Create a snapshot of vol1. + * State: + * esnap <-- vol1 + */ + vbdev_lvol_create_bdev_clone(uuid_esnap, lvs, "vol1", + lvol_op_with_handle_cb, clear_owh(&owh_data)); + poll_error_updated(&owh_data.lvserrno); + SPDK_CU_ASSERT_FATAL(owh_data.lvserrno == 0); + SPDK_CU_ASSERT_FATAL(owh_data.u.lvol != NULL); + vol1 = owh_data.u.lvol; + + /* Create a snapshot of vol1. + * State: + * esnap <-- vol2 <-- vol1 + */ + vbdev_lvol_create_snapshot(vol1, "vol2", lvol_op_with_handle_cb, clear_owh(&owh_data)); + poll_error_updated(&owh_data.lvserrno); + SPDK_CU_ASSERT_FATAL(owh_data.lvserrno == 0); + SPDK_CU_ASSERT_FATAL(owh_data.u.lvol != NULL); + vol2 = owh_data.u.lvol; + + /* Create a clone of vol2. + * State: + * esnap <-- vol2 <-- vol1 + * `---- vol3 + */ + vbdev_lvol_create_clone(vol2, "vol3", lvol_op_with_handle_cb, clear_owh(&owh_data)); + poll_error_updated(&owh_data.lvserrno); + SPDK_CU_ASSERT_FATAL(owh_data.lvserrno == 0); + SPDK_CU_ASSERT_FATAL(owh_data.u.lvol != NULL); + vol3 = owh_data.u.lvol; + + /* Unload the lvstore and delete esnap */ + rc = rc2 = 0xbad; + bdev_aio_delete("aio1", unregister_cb, &rc); + CU_ASSERT(spdk_bdev_get_by_name(uuid_esnap) != NULL) + delete_malloc_disk(malloc_bdev->name, unregister_cb, &rc2); + malloc_bdev = NULL; + poll_error_updated(&rc); + poll_error_updated(&rc2); + SPDK_CU_ASSERT_FATAL(rc == 0); + SPDK_CU_ASSERT_FATAL(rc2 == 0); + + /* Trigger the reload of the lvstore. + * State: + * (missing) <-- vol2 <-- vol1 + * `---- vol3 + */ + rc = create_aio_bdev("aio1", aiopath, bs_block_size, false); + SPDK_CU_ASSERT_FATAL(rc == 0); + rc = 0xbad; + spdk_bdev_wait_for_examine(esnap_wait_for_examine, &rc); + poll_error_updated(&rc); + + /* Verify vol1 is as described in diagram above */ + CU_ASSERT(spdk_bdev_get_by_name("lvs1/vol1") == NULL); + vol1 = spdk_lvol_get_by_names("lvs1", "vol1"); + SPDK_CU_ASSERT_FATAL(vol1 != NULL); + lvs = vol1->lvol_store; + CU_ASSERT(spdk_blob_is_clone(vol1->blob)); + CU_ASSERT(!spdk_blob_is_esnap_clone(vol1->blob)); + CU_ASSERT(!spdk_blob_is_snapshot(vol1->blob)); + CU_ASSERT(vol1->degraded_set == NULL); + + /* Verify vol2 is as described in diagram above */ + CU_ASSERT(spdk_bdev_get_by_name("lvs1/vol2") == NULL); + vol2 = spdk_lvol_get_by_names("lvs1", "vol2"); + SPDK_CU_ASSERT_FATAL(vol2 != NULL); + CU_ASSERT(!spdk_blob_is_clone(vol2->blob)); + CU_ASSERT(spdk_blob_is_esnap_clone(vol2->blob)); + CU_ASSERT(spdk_blob_is_snapshot(vol2->blob)); + CU_ASSERT(RB_MIN(degraded_lvol_sets_tree, &lvs->degraded_lvol_sets_tree) == vol2->degraded_set); + CU_ASSERT(TAILQ_FIRST(&vol2->degraded_set->lvols) == vol2); + + /* Verify vol3 is as described in diagram above */ + CU_ASSERT(spdk_bdev_get_by_name("lvs1/vol3") == NULL); + vol3 = spdk_lvol_get_by_names("lvs1", "vol3"); + SPDK_CU_ASSERT_FATAL(vol3 != NULL); + CU_ASSERT(spdk_blob_is_clone(vol3->blob)); + CU_ASSERT(!spdk_blob_is_esnap_clone(vol3->blob)); + CU_ASSERT(!spdk_blob_is_snapshot(vol3->blob)); + CU_ASSERT(vol3->degraded_set == NULL); + + /* Try to delete vol2. Should fail because it has multiple clones. */ + rc = 0xbad; + vbdev_lvol_destroy(vol2, lvol_op_complete_cb, &rc); + poll_error_updated(&rc); + CU_ASSERT(rc == -EPERM); + + /* Delete vol1 + * New state: + * (missing) <-- vol2 <-- vol3 + */ + rc = 0xbad; + vbdev_lvol_destroy(vol1, lvol_op_complete_cb, &rc); + poll_error_updated(&rc); + CU_ASSERT(rc == 0); + + /* Verify vol1 is gone */ + CU_ASSERT(spdk_bdev_get_by_name("lvs1/vol1") == NULL); + vol1 = spdk_lvol_get_by_names("lvs1", "vol1"); + CU_ASSERT(vol1 == NULL); + + /* Verify vol2 is as described in diagram above */ + CU_ASSERT(spdk_bdev_get_by_name("lvs1/vol2") == NULL); + vol2 = spdk_lvol_get_by_names("lvs1", "vol2"); + SPDK_CU_ASSERT_FATAL(vol2 != NULL); + CU_ASSERT(!spdk_blob_is_clone(vol2->blob)); + CU_ASSERT(spdk_blob_is_esnap_clone(vol2->blob)); + CU_ASSERT(spdk_blob_is_snapshot(vol2->blob)); + CU_ASSERT(RB_MIN(degraded_lvol_sets_tree, &lvs->degraded_lvol_sets_tree) == vol2->degraded_set); + CU_ASSERT(TAILQ_FIRST(&vol2->degraded_set->lvols) == vol2); + + /* Verify vol3 is as described in diagram above */ + CU_ASSERT(spdk_bdev_get_by_name("lvs1/vol3") == NULL); + vol3 = spdk_lvol_get_by_names("lvs1", "vol3"); + SPDK_CU_ASSERT_FATAL(vol3 != NULL); + CU_ASSERT(spdk_blob_is_clone(vol3->blob)); + CU_ASSERT(!spdk_blob_is_esnap_clone(vol3->blob)); + CU_ASSERT(!spdk_blob_is_snapshot(vol3->blob)); + CU_ASSERT(vol3->degraded_set == NULL); + + /* Delete vol2 + * New state: + * (missing) <-- vol3 + */ + rc = 0xbad; + vbdev_lvol_destroy(vol2, lvol_op_complete_cb, &rc); + poll_error_updated(&rc); + CU_ASSERT(rc == 0); + + /* Verify vol2 is gone */ + CU_ASSERT(spdk_bdev_get_by_name("lvs1/vol2") == NULL); + vol2 = spdk_lvol_get_by_names("lvs1", "vol2"); + SPDK_CU_ASSERT_FATAL(vol2 == NULL); + + /* Verify vol3 is as described in diagram above */ + CU_ASSERT(spdk_bdev_get_by_name("lvs1/vol3") == NULL); + vol3 = spdk_lvol_get_by_names("lvs1", "vol3"); + SPDK_CU_ASSERT_FATAL(vol3 != NULL); + CU_ASSERT(!spdk_blob_is_clone(vol3->blob)); + CU_ASSERT(spdk_blob_is_esnap_clone(vol3->blob)); + CU_ASSERT(!spdk_blob_is_snapshot(vol3->blob)); + CU_ASSERT(RB_MIN(degraded_lvol_sets_tree, &lvs->degraded_lvol_sets_tree) == vol3->degraded_set); + CU_ASSERT(TAILQ_FIRST(&vol3->degraded_set->lvols) == vol3); + + /* Delete vol3 + * New state: + * (nothing) + */ + rc = 0xbad; + vbdev_lvol_destroy(vol3, lvol_op_complete_cb, &rc); + poll_error_updated(&rc); + CU_ASSERT(rc == 0); + + /* Verify vol3 is gone */ + CU_ASSERT(spdk_bdev_get_by_name("lvs1/vol3") == NULL); + vol3 = spdk_lvol_get_by_names("lvs1", "vol3"); + SPDK_CU_ASSERT_FATAL(vol3 == NULL); + + /* Nothing depends on the missing bdev, so it is no longer missing. */ + CU_ASSERT(RB_EMPTY(&lvs->degraded_lvol_sets_tree)); + + /* Clean up */ + rc = rc2 = 0xbad; + bdev_aio_delete("aio1", unregister_cb, &rc); + poll_error_updated(&rc); + CU_ASSERT(rc == 0); + if (malloc_bdev != NULL) { + delete_malloc_disk(malloc_bdev->name, unregister_cb, &rc2); + poll_threads(); + CU_ASSERT(rc2 == 0); + } + rc = unlink(aiopath); + CU_ASSERT(rc == 0); +} + static void bdev_init_cb(void *arg, int rc) { @@ -478,6 +704,7 @@ main(int argc, char **argv) CU_ADD_TEST(suite, esnap_clone_io); CU_ADD_TEST(suite, esnap_hotplug); + CU_ADD_TEST(suite, esnap_remove_degraded); allocate_threads(2); set_thread(0); diff --git a/test/lvol/external_snapshot.sh b/test/lvol/external_snapshot.sh index 87130e245..f9719a71f 100755 --- a/test/lvol/external_snapshot.sh +++ b/test/lvol/external_snapshot.sh @@ -407,6 +407,58 @@ function test_esnap_late_arrival() { rpc_cmd bdev_malloc_delete "$esnap_dev" } +function test_esnap_remove_degraded() { + local bs_dev esnap_dev + local block_size=512 + local esnap_size_mb=1 + local lvs_cluster_size=$((16 * 1024)) + local lvs_uuid esnap_uuid eclone_uuid snap_uuid clone_uuid uuid + local aio_bdev=test_esnap_reload_aio0 + local lvols + + # Create the lvstore on an aio device. Can't use malloc because we need to remove + # the device and re-add it to trigger an lvstore unload and then load. + rm -f $testdir/aio_bdev_0 + truncate -s "${AIO_SIZE_MB}M" $testdir/aio_bdev_0 + bs_dev=$(rpc_cmd bdev_aio_create "$testdir/aio_bdev_0" "$aio_bdev" "$block_size") + lvs_uuid=$(rpc_cmd bdev_lvol_create_lvstore -c "$lvs_cluster_size" "$bs_dev" lvs_test) + + # Create a bdev that will be the external snapshot + esnap_uuid=e4b40d8b-f623-416d-8234-baf5a4c83cbd + esnap_dev=$(rpc_cmd bdev_malloc_create -u "$esnap_uuid" "$esnap_size_mb" "$block_size") + eclone_uuid=$(rpc_cmd bdev_lvol_clone_bdev "$esnap_uuid" lvs_test "eclone") + rpc_cmd bdev_get_bdevs -b "$eclone_uuid" + + # Create a clone of eclone + rpc_cmd bdev_lvol_set_read_only "$eclone_uuid" + clone_uuid=$(rpc_cmd bdev_lvol_clone "$eclone_uuid" clone) + rpc_cmd bdev_get_bdevs -b "$clone_uuid" + + # Reload the lvolstore without the external snapshot + rpc_cmd bdev_aio_delete "$aio_bdev" + NOT rpc_cmd bdev_lvol_get_lvstores -l lvs_test + rpc_cmd bdev_malloc_delete "$esnap_dev" + bs_dev=$(rpc_cmd bdev_aio_create "$testdir/aio_bdev_0" "$aio_bdev" "$block_size") + lvs_uuid=$(rpc_cmd bdev_lvol_get_lvstores -l lvs_test) + + # Verify clone and eclone are degraded + lvols=$(rpc_cmd bdev_lvol_get_lvols) + [[ "$(jq -r '.[] | select(.uuid == "'$eclone_uuid'").is_degraded' <<< "$lvols")" == "true" ]] + [[ "$(jq -r '.[] | select(.uuid == "'$clone_uuid'").is_degraded' <<< "$lvols")" == "true" ]] + NOT rpc_cmd bdev_get_bdevs -b "$clone_uuid" + NOT rpc_cmd bdev_get_bdevs -b "$eclone_uuid" + + # Delete the lvols and verify they are gone. + rpc_cmd bdev_lvol_delete "$clone_uuid" + lvols=$(rpc_cmd bdev_lvol_get_lvols) + [[ "$(jq -r '. | length' <<< "$lvols")" == "1" ]] + rpc_cmd bdev_lvol_delete "$eclone_uuid" + lvols=$(rpc_cmd bdev_lvol_get_lvols) + [[ "$(jq -r '. | length' <<< "$lvols")" == "0" ]] + + rpc_cmd bdev_aio_delete "$aio_bdev" +} + $SPDK_BIN_DIR/spdk_tgt & spdk_pid=$! trap 'killprocess "$spdk_pid"; rm -f "$testdir/aio_bdev_0"; exit 1' SIGINT SIGTERM SIGPIPE EXIT @@ -417,6 +469,7 @@ run_test "test_esnap_reload" test_esnap_reload run_test "test_esnap_reload" test_esnap_reload_missing run_test "test_esnap_clones" test_esnap_clones run_test "test_esnap_late_arrival" test_esnap_late_arrival +run_test "test_esnap_remove_degraded" test_esnap_remove_degraded trap - SIGINT SIGTERM SIGPIPE EXIT killprocess $spdk_pid