Commit d3594f84 authored by xupeng-mingtu's avatar xupeng-mingtu Committed by Konrad Sztyber
Browse files

lib/blob: fix data inconsistency when unmap a thin-provisioned blob that is backed 



The issue may happen in this case:
	1.create lvol A from snapshot B
	2.write data to A_cluster1
	3.unmap A_cluster1
	4.read data from lba coverd by A_cluster1
the result of step 4 should be zero, but it’s actually the data in B_cluster1

Change-Id: If08bdf392d8d3dee737fcacca03bd9f04faa3933
Signed-off-by: default avatarxupeng-mingtu <jingmamour@gmail.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/22325


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarJim Harris <jim.harris@samsung.com>
Reviewed-by: default avatarTomasz Zawadzki <tomasz@tzawadzki.com>
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarMateusz Kozlowski <mateusz.kozlowski@solidigm.com>
parent 05632afd

lib/blob/blobstore.c

+1 −0
Original line number Diff line number Diff line
@@ -3205,6 +3205,7 @@ blob_request_submit_op_single(struct spdk_io_channel *_ch, struct spdk_blob *blo 


		/* if aligned with cluster release cluster */

		if (spdk_blob_is_thin_provisioned(blob) && is_allocated &&

		    blob_backed_with_zeroes_dev(blob) &&

		    bs_io_units_per_cluster(blob) == length) {

			struct spdk_bs_channel *bs_channel = spdk_io_channel_get_ctx(_ch);

			uint32_t cluster_start_page;

lib/blob/blobstore.h

+1 −0
Original line number Diff line number Diff line
@@ -435,6 +435,7 @@ struct spdk_bs_dev *bs_create_zeroes_dev(void); 
struct spdk_bs_dev *bs_create_blob_bs_dev(struct spdk_blob *blob);

struct spdk_io_channel *blob_esnap_get_io_channel(struct spdk_io_channel *ch,

		struct spdk_blob *blob);

bool blob_backed_with_zeroes_dev(struct spdk_blob *blob);



/* Unit Conversions

 *

lib/blob/zeroes.c

+6 −0
Original line number Diff line number Diff line
@@ -160,3 +160,9 @@ bs_create_zeroes_dev(void) 
{

	return &g_zeroes_bs_dev;

}



bool

blob_backed_with_zeroes_dev(struct spdk_blob *blob)

{

	return blob != NULL && blob->back_bs_dev == &g_zeroes_bs_dev;

}

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

+58 −1
Original line number Diff line number Diff line
@@ -4666,7 +4666,7 @@ static void 
blob_thin_prov_unmap_cluster(void)

{

	struct spdk_blob_store *bs;

	struct spdk_blob *blob;

	struct spdk_blob *blob, *snapshot;

	struct spdk_io_channel *ch;

	struct spdk_bs_dev *dev;

	struct spdk_bs_opts bs_opts;
@@ -4677,6 +4677,7 @@ blob_thin_prov_unmap_cluster(void) 
	uint8_t payload_read[4096];

	const uint32_t CLUSTER_COUNT = 3;

	uint32_t pages_per_cluster;

	spdk_blob_id blobid, snapshotid;

	uint32_t i;

	int err;
@@ -4706,6 +4707,7 @@ blob_thin_prov_unmap_cluster(void) 
	blob = ut_blob_create_and_open(bs, &opts);

	CU_ASSERT(free_clusters == CLUSTER_COUNT);

	CU_ASSERT(free_clusters == spdk_bs_free_cluster_count(bs));

	blobid = spdk_blob_get_id(blob);



	g_bserrno = -1;

	spdk_blob_resize(blob, CLUSTER_COUNT, blob_op_complete, NULL);
@@ -4836,7 +4838,62 @@ blob_thin_prov_unmap_cluster(void) 
	CU_ASSERT(err == 0);

	CU_ASSERT(1 == spdk_bs_free_cluster_count(bs));



	/* Test thin-provisioned blob that is backed */

	spdk_blob_resize(blob, 1, blob_op_complete, NULL);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);

	spdk_blob_sync_md(blob, blob_op_complete, NULL);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);

	CU_ASSERT(free_clusters == spdk_bs_free_cluster_count(bs));



	g_bserrno = -1;

	memset(payload_write, 1, sizeof(payload_write));

	spdk_blob_io_write(blob, ch, payload_write, 0, 1, blob_op_complete, NULL);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);

	CU_ASSERT(free_clusters - 1 == spdk_bs_free_cluster_count(bs));



	/* Create a snapshot */

	CU_ASSERT_EQUAL(_get_snapshots_count(bs), 0);

	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);

	CU_ASSERT_EQUAL(_get_snapshots_count(bs), 1);

	snapshotid = g_blobid;

	CU_ASSERT(free_clusters - 1 == spdk_bs_free_cluster_count(bs));

	spdk_bs_open_blob(bs, snapshotid, blob_op_with_handle_complete, NULL);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);

	SPDK_CU_ASSERT_FATAL(g_blob != NULL);

	snapshot = g_blob;



	/* Write data to blob, it will alloc new cluster */

	g_bserrno = -1;

	memset(payload_write, 2, sizeof(payload_write));

	spdk_blob_io_write(blob, ch, payload_write, 0, 1, blob_op_complete, NULL);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);

	CU_ASSERT(free_clusters - 2 == spdk_bs_free_cluster_count(bs));



	/* Unmap one whole cluster, but do not release this cluster */

	g_bserrno = -1;

	spdk_blob_io_unmap(blob, ch, 0, pages_per_cluster, blob_op_complete, NULL);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);

	CU_ASSERT(free_clusters - 2 == spdk_bs_free_cluster_count(bs));



	/* Verify the data read from the cluster is zeroed out */

	g_bserrno = -1;

	memset(payload_write, 0, sizeof(payload_write));

	spdk_blob_io_read(blob, ch, payload_read, 0, 1, blob_op_complete, NULL);

	poll_threads();

	CU_ASSERT(g_bserrno == 0);

	CU_ASSERT(memcmp(payload_write, payload_read, 4096) == 0);



	ut_blob_close_and_delete(bs, blob);

	ut_blob_close_and_delete(bs, snapshot);

	CU_ASSERT(free_clusters == spdk_bs_free_cluster_count(bs));



	spdk_bs_free_io_channel(ch);