Commit 7f007b44 authored by Tomasz Zawadzki's avatar Tomasz Zawadzki
Browse files

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: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
Change-Id: I495452c578d879f749281cebf8975eb2c1c7f79a
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2057


Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
parent 519d7209

lib/blob/blobstore.c

+16 −0
Original line number Diff line number Diff line
@@ -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;

	}

test/unit/lib/blob/blob.c/blob_ut.c

+40 −0
Original line number Diff line number Diff line
@@ -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);