From a512214517a512d44aa7c62a963636b57b23faa5 Mon Sep 17 00:00:00 2001 From: Tomasz Zawadzki Date: Thu, 15 Apr 2021 07:37:20 -0400 Subject: [PATCH] lib/blob: update extent pages during snapshot deletion When both clone and snapshot had already extent pages corresponding to the same region in cluster map, the clone extent page was replaced with one from snapshot. This was incorrect and would result in loss of clusters from clones extent page. It did not occur in practice because all extent pages were rewritten anyway during md sync. Cluster map was correct so updated extent pages were too. Cluster map correctness is verified in UT _blob_inflate_rw(true), at the very end when checking data consistency of inflated blob. This patch writes out the updated extent page explicitly. So it would be possible to skip wirting out extent pages during md sync later in the series. Note 1) At this point in series the extent page is written here, and in blob persists. The later will be removed later in series. Note 2) Errors during updating extent pages are not accounted for, but neither does syncing them in blob persist. Signed-off-by: Tomasz Zawadzki Change-Id: I7deac3c64299f33f8df49e860af1a16295c074e6 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/7438 Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Ben Walker Community-CI: Mellanox Build Bot --- lib/blob/blobstore.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/lib/blob/blobstore.c b/lib/blob/blobstore.c index f0707d31a..d27f1c328 100644 --- a/lib/blob/blobstore.c +++ b/lib/blob/blobstore.c @@ -6346,6 +6346,7 @@ struct delete_snapshot_ctx { spdk_blob_op_with_handle_complete cb_fn; void *cb_arg; int bserrno; + uint32_t next_extent_page; }; static void @@ -6540,15 +6541,33 @@ delete_snapshot_update_extent_pages_cpl(struct delete_snapshot_ctx *ctx) } static void -delete_snapshot_update_extent_pages(struct delete_snapshot_ctx *ctx) +delete_snapshot_update_extent_pages(void *cb_arg, int bserrno) { + struct delete_snapshot_ctx *ctx = cb_arg; + uint32_t *extent_page; uint64_t i; - for (i = 0; i < ctx->snapshot->active.num_extent_pages && + for (i = ctx->next_extent_page; 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]; + if (ctx->snapshot->active.extent_pages[i] == 0) { + /* No extent page to use from snapshot */ + continue; } + + extent_page = &ctx->clone->active.extent_pages[i]; + if (*extent_page == 0) { + /* Copy extent page from snapshot when clone did not have a matching one */ + *extent_page = ctx->snapshot->active.extent_pages[i]; + continue; + } + + /* Clone and snapshot both contain partialy filled matching extent pages. + * Update the clone extent page in place with cluster map containing the mix of both. */ + ctx->next_extent_page = i + 1; + + blob_write_extent_page(ctx->clone, *extent_page, i * SPDK_EXTENTS_PER_EP, + delete_snapshot_update_extent_pages, ctx); + return; } delete_snapshot_update_extent_pages_cpl(ctx); } @@ -6576,7 +6595,8 @@ delete_snapshot_sync_snapshot_xattr_cpl(void *cb_arg, int bserrno) ctx->clone->active.clusters[i] = ctx->snapshot->active.clusters[i]; } } - delete_snapshot_update_extent_pages(ctx); + ctx->next_extent_page = 0; + delete_snapshot_update_extent_pages(ctx, 0); } static void