Commit 68063cd8 authored by Tomasz Zawadzki's avatar Tomasz Zawadzki Committed by Jim Harris
Browse files

lib/blob: force md update during decouple parent 



Fixes #1933

When decoupling parent the updated parent_id was
not persisted to the blob if it was a snapshot.
Due to having md_ro set to true, blob_set_xattr()
failed.

Later on the incorrect parent_id could cause troubles
like in the github issue, when deleting that snapshot.

This patch adds return code check for blob_set_xattr
and forces md_ro to false during blob md sync.

Since some of code paths are shared between decouple,
inflate and clone operations, the final callback for them
is doing revert of the original md_ro.

Signed-off-by: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
Change-Id: If017455f72e4d809fe533d9f986e5ae6bb8e2035
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8420


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
parent 7478e053

lib/blob/blobstore.c

+15 −2
Original line number Diff line number Diff line
@@ -5561,6 +5561,7 @@ struct spdk_clone_snapshot_ctx { 
	struct {

		spdk_blob_id id;

		struct spdk_blob *blob;

		bool md_ro;

	} original;

	struct {

		spdk_blob_id id;
@@ -5618,6 +5619,9 @@ bs_snapshot_unfreeze_cpl(void *cb_arg, int bserrno) 
	ctx->original.id = origblob->id;

	origblob->locked_operation_in_progress = false;



	/* Revert md_ro to original state */

	origblob->md_ro = ctx->original.md_ro;



	spdk_blob_close(origblob, bs_clone_snapshot_cleanup_finish, ctx);

}
@@ -5982,6 +5986,7 @@ bs_clone_origblob_open_cpl(void *cb_arg, struct spdk_blob *_blob, int bserrno) 
	}



	ctx->original.blob = _blob;

	ctx->original.md_ro = _blob->md_ro;



	if (!_blob->data_ro || !_blob->md_ro) {

		SPDK_DEBUGLOG(blob, "Clone not from read-only blob\n");
@@ -6056,12 +6061,19 @@ bs_inflate_blob_set_parent_cpl(void *cb_arg, struct spdk_blob *_parent, int bser 
		return;

	}



	/* Temporarily override md_ro flag for MD modification */

	_blob->md_ro = false;



	bserrno = blob_set_xattr(_blob, BLOB_SNAPSHOT, &_parent->id, sizeof(spdk_blob_id), true);

	if (bserrno != 0) {

		bs_clone_snapshot_origblob_cleanup(ctx, bserrno);

		return;

	}



	assert(_parent != NULL);



	bs_blob_list_remove(_blob);

	_blob->parent_id = _parent->id;

	blob_set_xattr(_blob, BLOB_SNAPSHOT, &_blob->parent_id,

		       sizeof(spdk_blob_id), true);



	_blob->back_bs_dev->destroy(_blob->back_bs_dev);

	_blob->back_bs_dev = bs_create_blob_bs_dev(_parent);
@@ -6171,6 +6183,7 @@ bs_inflate_blob_open_cpl(void *cb_arg, struct spdk_blob *_blob, int bserrno) 
	}



	ctx->original.blob = _blob;

	ctx->original.md_ro = _blob->md_ro;



	if (_blob->locked_operation_in_progress) {

		SPDK_DEBUGLOG(blob, "Cannot inflate blob - another operation in progress\n");

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

+108 −0
Original line number Diff line number Diff line
@@ -5481,6 +5481,113 @@ blob_relations2(void) 
	g_bs = NULL;

}



/**

 * Snapshot-clones relation test 3

 *

 *         snapshot0

 *            |

 *         snapshot1

 *            |

 *         snapshot2

 *            |

 *           blob

 */

static void

blob_relations3(void)

{

	struct spdk_blob_store *bs;

	struct spdk_bs_dev *dev;

	struct spdk_io_channel *channel;

	struct spdk_bs_opts bs_opts;

	struct spdk_blob_opts opts;

	struct spdk_blob *blob;

	spdk_blob_id blobid, snapshotid0, snapshotid1, snapshotid2;



	dev = init_dev();

	spdk_bs_opts_init(&bs_opts, sizeof(opts));

	snprintf(bs_opts.bstype.bstype, sizeof(bs_opts.bstype.bstype), "TESTTYPE");



	spdk_bs_init(dev, &bs_opts, bs_op_with_handle_complete, NULL);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);

	SPDK_CU_ASSERT_FATAL(g_bs != NULL);

	bs = g_bs;



	channel = spdk_bs_alloc_io_channel(bs);

	SPDK_CU_ASSERT_FATAL(channel != NULL);



	/* 1. Create blob with 10 clusters */

	ut_spdk_blob_opts_init(&opts);

	opts.num_clusters = 10;



	blob = ut_blob_create_and_open(bs, &opts);

	blobid = spdk_blob_get_id(blob);



	/* 2. Create snapshot0 */

	spdk_bs_create_snapshot(bs, blobid, NULL, blob_op_with_id_complete, NULL);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);

	CU_ASSERT(g_blobid != SPDK_BLOBID_INVALID);

	snapshotid0 = g_blobid;



	/* 3. Create snapshot1 */

	spdk_bs_create_snapshot(bs, blobid, NULL, blob_op_with_id_complete, NULL);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);

	CU_ASSERT(g_blobid != SPDK_BLOBID_INVALID);

	snapshotid1 = g_blobid;



	/* 4. Create snapshot2 */

	spdk_bs_create_snapshot(bs, blobid, NULL, blob_op_with_id_complete, NULL);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);

	CU_ASSERT(g_blobid != SPDK_BLOBID_INVALID);

	snapshotid2 = g_blobid;



	/* 5. Decouple blob */

	spdk_bs_blob_decouple_parent(bs, channel, blobid, blob_op_complete, NULL);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);



	/* 6. Decouple snapshot2. Make sure updating md of snapshot2 is possible */

	spdk_bs_blob_decouple_parent(bs, channel, snapshotid2, blob_op_complete, NULL);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);



	/* 7. Delete blob */

	spdk_blob_close(blob, blob_op_complete, NULL);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);



	spdk_bs_delete_blob(bs, blobid, blob_op_complete, NULL);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);



	/* 8. Delete snapshot2.

	 * If md of snapshot 2 was updated, it should be possible to delete it */

	spdk_bs_delete_blob(bs, snapshotid2, blob_op_complete, NULL);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);



	/* Remove remaining blobs and unload bs */

	spdk_bs_delete_blob(bs, snapshotid1, blob_op_complete, NULL);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);



	spdk_bs_delete_blob(bs, snapshotid0, blob_op_complete, NULL);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);



	spdk_bs_free_io_channel(channel);

	poll_threads();



	spdk_bs_unload(bs, bs_op_complete, NULL);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);



	g_bs = NULL;

}



static void

blobstore_clean_power_failure(void)

{
@@ -6924,6 +7031,7 @@ int main(int argc, char **argv) 
	CU_ADD_TEST(suite_bs, blob_snapshot_rw_iov);

	CU_ADD_TEST(suite, blob_relations);

	CU_ADD_TEST(suite, blob_relations2);

	CU_ADD_TEST(suite, blob_relations3);

	CU_ADD_TEST(suite, blobstore_clean_power_failure);

	CU_ADD_TEST(suite, blob_delete_snapshot_power_failure);

	CU_ADD_TEST(suite, blob_create_snapshot_power_failure);