vbdev_lvol: allow degraded lvols to be deleted

An esnap clone is now deletable when its external snapshot is missing.
Likewise, the tree of degraded lvols rooted at a degraded esnap clone
can also be deleted, subject to the normal restrictions.

Signed-off-by: Mike Gerdts <mgerdts@nvidia.com>
Change-Id: I711ae25d57f5625a955d1f4cdb2839dd0a6cb095
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17549
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>

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);
 }
 

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) {
+	if (bdev != NULL) {
-		SPDK_ERRLOG("no bdev for provided name %s\n", req.name);
+		lvol = vbdev_lvol_get_from_bdev(bdev);
-		spdk_jsonrpc_send_error_response(request, -ENODEV, spdk_strerror(ENODEV));
+		if (lvol != NULL) {
-		goto cleanup;
+			goto done;
+		}
 	}
 
-	lvol = vbdev_lvol_get_from_bdev(bdev);
+	/* lvol is degraded, get lvol via UUID */
-	if (lvol == NULL) {
+	if (spdk_uuid_parse(&uuid, req.name) == 0) {
-		spdk_jsonrpc_send_error_response(request, -ENODEV, spdk_strerror(ENODEV));
+		lvol = spdk_lvol_get_by_uuid(&uuid);
-		goto cleanup;
+		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:

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);

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