From e8ddb060f87252c596adbd4ccb492df7e2f7925d Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Fri, 4 May 2018 01:41:21 -0700 Subject: [PATCH] blob: don't try to claim cluster 0 in recovery code Thin provisioned blobs mark unallocated clusters with cluster ID 0. During recovery from a dirty shutdown, we must not try to claim cluster 0 - we should ignore them instead. Fixes issue #291. Signed-off-by: Jim Harris Change-Id: If0dd42416f5de8d9972073bf6ed44eb8bc655415 Reviewed-on: https://review.gerrithub.io/410065 Reviewed-by: Daniel Verkamp Reviewed-by: Ben Walker Tested-by: SPDK Automated Test System --- lib/blob/blobstore.c | 16 ++++++++++++---- test/unit/lib/blob/blob.c/blob_ut.c | 7 ++++--- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/lib/blob/blobstore.c b/lib/blob/blobstore.c index 2de4af703d..6bdd16c826 100644 --- a/lib/blob/blobstore.c +++ b/lib/blob/blobstore.c @@ -2648,16 +2648,24 @@ _spdk_bs_load_replay_md_parse_page(const struct spdk_blob_md_page *page, struct struct spdk_blob_md_descriptor_extent *desc_extent; unsigned int i, j; unsigned int cluster_count = 0; + uint32_t cluster_idx; desc_extent = (struct spdk_blob_md_descriptor_extent *)desc; for (i = 0; i < desc_extent->length / sizeof(desc_extent->extents[0]); i++) { for (j = 0; j < desc_extent->extents[i].length; j++) { - spdk_bit_array_set(bs->used_clusters, desc_extent->extents[i].cluster_idx + j); - if (bs->num_free_clusters == 0) { - return -1; + cluster_idx = desc_extent->extents[i].cluster_idx; + /* + * cluster_idx = 0 means an unallocated cluster - don't mark that + * in the used cluster map. + */ + if (cluster_idx != 0) { + spdk_bit_array_set(bs->used_clusters, cluster_idx + j); + if (bs->num_free_clusters == 0) { + return -1; + } + bs->num_free_clusters--; } - bs->num_free_clusters--; cluster_count++; } } diff --git a/test/unit/lib/blob/blob.c/blob_ut.c b/test/unit/lib/blob/blob.c/blob_ut.c index b737154e77..4c3947812c 100644 --- a/test/unit/lib/blob/blob.c/blob_ut.c +++ b/test/unit/lib/blob/blob.c/blob_ut.c @@ -524,9 +524,10 @@ blob_thin_provision(void) spdk_blob_close(blob, blob_op_complete, NULL); CU_ASSERT(g_bserrno == 0); - spdk_bs_unload(g_bs, bs_op_complete, NULL); - CU_ASSERT(g_bserrno == 0); - g_bs = NULL; + /* Do not shut down cleanly. This makes sure that when we load again + * and try to recover a valid used_cluster map, that blobstore will + * ignore clusters with index 0 since these are unallocated clusters. + */ /* Load an existing blob store and check if invalid_flags is set */ dev = init_dev();