From 0f5157377f3df82bd753e9e013352ceafa6055a2 Mon Sep 17 00:00:00 2001 From: Tomasz Zawadzki Date: Thu, 5 Mar 2020 10:02:13 -0500 Subject: [PATCH] lib/blob: merge EP of a clone when deleting a snapshot In general it is not possible to delete snapshot when there are clones on top of it. There is special case when there is just a single clone on top that snapshot. In such case the clone is 'merged' with snapshot. Unallocated clusters in clone, are filled with the ones in snapshot (if allocated there). Similar behavior should have occurred for extent pages. This patch adds the implementation for moving EP from snapshot to clone along with UT. The UT exposes the issue by allowing delete_blob to proceed beyond just unrecoverable snapshot blob. Fixes #1291 Signed-off-by: Tomasz Zawadzki Change-Id: Ib2824c5737021f8e8d9b533a4cd245c12e6fe9fa Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1163 Reviewed-by: Jim Harris Reviewed-by: Ben Walker Tested-by: SPDK CI Jenkins --- lib/blob/blobstore.c | 13 +++++++++++++ test/unit/lib/blob/blob.c/blob_ut.c | 9 ++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/lib/blob/blobstore.c b/lib/blob/blobstore.c index 499d4be40..8b253dec6 100644 --- a/lib/blob/blobstore.c +++ b/lib/blob/blobstore.c @@ -6212,7 +6212,14 @@ _spdk_delete_snapshot_sync_clone_cpl(void *cb_arg, int bserrno) ctx->snapshot->active.clusters[i] = 0; } } + for (i = 0; i < ctx->snapshot->active.num_extent_pages && + i < ctx->clone->active.num_extent_pages; i++) { + if (ctx->clone->active.extent_pages[i] == ctx->snapshot->active.extent_pages[i]) { + ctx->snapshot->active.extent_pages[i] = 0; + } + } + _spdk_blob_set_thin_provision(ctx->snapshot); ctx->snapshot->state = SPDK_BLOB_STATE_DIRTY; if (ctx->parent_snapshot_entry != NULL) { @@ -6245,6 +6252,12 @@ _spdk_delete_snapshot_sync_snapshot_xattr_cpl(void *cb_arg, int bserrno) ctx->clone->active.clusters[i] = ctx->snapshot->active.clusters[i]; } } + for (i = 0; i < ctx->snapshot->active.num_extent_pages && + i < ctx->clone->active.num_extent_pages; i++) { + if (ctx->clone->active.extent_pages[i] == 0) { + ctx->clone->active.extent_pages[i] = ctx->snapshot->active.extent_pages[i]; + } + } /* Delete old backing bs_dev from clone (related to snapshot that will be removed) */ ctx->clone->back_bs_dev->destroy(ctx->clone->back_bs_dev); diff --git a/test/unit/lib/blob/blob.c/blob_ut.c b/test/unit/lib/blob/blob.c/blob_ut.c index caf902e9e..53bf681df 100644 --- a/test/unit/lib/blob/blob.c/blob_ut.c +++ b/test/unit/lib/blob/blob.c/blob_ut.c @@ -5223,6 +5223,7 @@ blob_delete_snapshot_power_failure(void) spdk_blob_id ids[3] = {}; int rc; bool deleted = false; + int delete_snapshot_bserrno = -1; thresholds.general_threshold = 1; while (!deleted) { @@ -5257,6 +5258,7 @@ blob_delete_snapshot_power_failure(void) spdk_bs_delete_blob(bs, snapshotid, blob_op_complete, NULL); poll_threads(); + delete_snapshot_bserrno = g_bserrno; /* Do not shut down cleanly. Assumption is that after snapshot deletion * reports success, changes to both blobs should already persisted. */ @@ -5294,7 +5296,12 @@ blob_delete_snapshot_power_failure(void) CU_ASSERT(g_bserrno == 0); } else { CU_ASSERT(spdk_blob_get_parent_snapshot(bs, blobid) == SPDK_BLOBID_INVALID); - deleted = true; + /* Snapshot might have been left in unrecoverable state, so it does not open. + * Yet delete might perform further changes to the clone after that. + * This UT should test until snapshot is deleted and delete call succeeds. */ + if (delete_snapshot_bserrno == 0) { + deleted = true; + } } spdk_blob_close(blob, blob_op_complete, NULL);