From 7f007b44a9895826ddc606bb5351d6f09f3924ef Mon Sep 17 00:00:00 2001 From: Tomasz Zawadzki Date: Mon, 27 Apr 2020 12:59:01 -0400 Subject: [PATCH] lib/blob: clear blobid and md_page on blob_create failure Blobid and md_page is claimed as first step of blob creation. If blob creation failed, both should returned to be used by other blobs. This caused multiple reports of: "Metadata page 1 crc mismatch" when loading blobstore due to md_pages not actually containing the written out md pages. Signed-off-by: Tomasz Zawadzki Change-Id: I495452c578d879f749281cebf8975eb2c1c7f79a Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2057 Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Ben Walker --- lib/blob/blobstore.c | 16 ++++++++++++ test/unit/lib/blob/blob.c/blob_ut.c | 40 +++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/lib/blob/blobstore.c b/lib/blob/blobstore.c index 29cdd5daa..1b64cb301 100644 --- a/lib/blob/blobstore.c +++ b/lib/blob/blobstore.c @@ -5142,6 +5142,12 @@ static void _spdk_bs_create_blob_cpl(spdk_bs_sequence_t *seq, void *cb_arg, int bserrno) { struct spdk_blob *blob = cb_arg; + uint32_t page_idx = _spdk_bs_blobid_to_page(blob->id); + + if (bserrno != 0) { + spdk_bit_array_clear(blob->bs->used_blobids, page_idx); + _spdk_bs_release_md_page(blob->bs, page_idx); + } _spdk_blob_free(blob); @@ -5203,6 +5209,8 @@ _spdk_bs_create_blob(struct spdk_blob_store *bs, blob = _spdk_blob_alloc(bs, id); if (!blob) { + spdk_bit_array_clear(bs->used_blobids, page_idx); + _spdk_bs_release_md_page(bs, page_idx); cb_fn(cb_arg, 0, -ENOMEM); return; } @@ -5225,6 +5233,8 @@ _spdk_bs_create_blob(struct spdk_blob_store *bs, rc = _spdk_blob_set_xattrs(blob, &opts->xattrs, false); if (rc < 0) { _spdk_blob_free(blob); + spdk_bit_array_clear(bs->used_blobids, page_idx); + _spdk_bs_release_md_page(bs, page_idx); cb_fn(cb_arg, 0, rc); return; } @@ -5232,6 +5242,8 @@ _spdk_bs_create_blob(struct spdk_blob_store *bs, rc = _spdk_blob_set_xattrs(blob, internal_xattrs, true); if (rc < 0) { _spdk_blob_free(blob); + spdk_bit_array_clear(bs->used_blobids, page_idx); + _spdk_bs_release_md_page(bs, page_idx); cb_fn(cb_arg, 0, rc); return; } @@ -5245,6 +5257,8 @@ _spdk_bs_create_blob(struct spdk_blob_store *bs, rc = _spdk_blob_resize(blob, opts->num_clusters); if (rc < 0) { _spdk_blob_free(blob); + spdk_bit_array_clear(bs->used_blobids, page_idx); + _spdk_bs_release_md_page(bs, page_idx); cb_fn(cb_arg, 0, rc); return; } @@ -5256,6 +5270,8 @@ _spdk_bs_create_blob(struct spdk_blob_store *bs, seq = bs_sequence_start(bs->md_channel, &cpl); if (!seq) { _spdk_blob_free(blob); + spdk_bit_array_clear(bs->used_blobids, page_idx); + _spdk_bs_release_md_page(bs, page_idx); cb_fn(cb_arg, 0, -ENOMEM); return; } diff --git a/test/unit/lib/blob/blob.c/blob_ut.c b/test/unit/lib/blob/blob.c/blob_ut.c index 0516ee747..f09169bea 100644 --- a/test/unit/lib/blob/blob.c/blob_ut.c +++ b/test/unit/lib/blob/blob.c/blob_ut.c @@ -407,6 +407,45 @@ blob_create(void) CU_ASSERT(g_bserrno == -ENOSPC); } +static void +blob_create_fail(void) +{ + struct spdk_blob_store *bs = g_bs; + struct spdk_blob_opts opts; + spdk_blob_id blobid; + uint32_t used_blobids_count = spdk_bit_array_count_set(bs->used_blobids); + uint32_t used_md_pages_count = spdk_bit_array_count_set(bs->used_md_pages); + + /* NULL callback */ + ut_spdk_blob_opts_init(&opts); + opts.xattrs.names = g_xattr_names; + opts.xattrs.get_value = NULL; + opts.xattrs.count = 1; + opts.xattrs.ctx = &g_ctx; + + blobid = spdk_bit_array_find_first_clear(bs->used_md_pages, 0); + spdk_bs_create_blob_ext(bs, &opts, blob_op_with_id_complete, NULL); + poll_threads(); + CU_ASSERT(g_bserrno == -EINVAL); + CU_ASSERT(g_blobid != SPDK_BLOBID_INVALID); + CU_ASSERT(spdk_bit_array_count_set(bs->used_blobids) == used_blobids_count); + CU_ASSERT(spdk_bit_array_count_set(bs->used_md_pages) == used_md_pages_count); + + spdk_bs_open_blob(bs, blobid, blob_op_with_handle_complete, NULL); + poll_threads(); + CU_ASSERT(g_bserrno == -ENOENT); + SPDK_CU_ASSERT_FATAL(g_blob == NULL); + + ut_bs_reload(&bs, NULL); + CU_ASSERT(spdk_bit_array_count_set(bs->used_blobids) == used_blobids_count); + CU_ASSERT(spdk_bit_array_count_set(bs->used_md_pages) == used_md_pages_count); + + spdk_bs_iter_first(bs, blob_op_with_handle_complete, NULL); + poll_threads(); + CU_ASSERT(g_blob == NULL); + CU_ASSERT(g_bserrno == -ENOENT); +} + static void blob_create_internal(void) { @@ -6573,6 +6612,7 @@ int main(int argc, char **argv) CU_ADD_TEST(suite, blob_init); CU_ADD_TEST(suite_bs, blob_open); CU_ADD_TEST(suite_bs, blob_create); + CU_ADD_TEST(suite_bs, blob_create_fail); CU_ADD_TEST(suite_bs, blob_create_internal); CU_ADD_TEST(suite, blob_thin_provision); CU_ADD_TEST(suite_bs, blob_snapshot);