From 8ca38e04b1d29bc97618b6470d65b526124efff0 Mon Sep 17 00:00:00 2001 From: Maciej Szwed Date: Wed, 12 Dec 2018 14:23:06 +0100 Subject: [PATCH] blob: Remove snpashot from the list only on blob deletion When last clone of a snapshot is being deleted we remove that snapshot from snapshots list. We should not do that as it still works as a snapshot and it is read-only, but it does not list as a snpashot from get_bdevs. Instead remove snapshot entry from the list when blob that represents that snapshot is being removed. Signed-off-by: Maciej Szwed Change-Id: I8d76229567fb0d9f15d29bad3fd94b9813249604 Reviewed-on: https://review.gerrithub.io/436971 (master) Reviewed-on: https://review.gerrithub.io/437194 Reviewed-by: Jim Harris Reviewed-by: Ben Walker Tested-by: SPDK CI Jenkins --- lib/blob/blobstore.c | 16 +++++++++----- test/unit/lib/blob/blob.c/blob_ut.c | 34 ++++++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/lib/blob/blobstore.c b/lib/blob/blobstore.c index 3b29418026..83be1b17cc 100644 --- a/lib/blob/blobstore.c +++ b/lib/blob/blobstore.c @@ -2361,11 +2361,6 @@ _spdk_bs_blob_list_remove(struct spdk_blob *blob) free(clone_entry); snapshot_entry->clone_count--; - if (snapshot_entry->clone_count == 0) { - /* Snapshot have no more clones */ - TAILQ_REMOVE(&blob->bs->snapshots, snapshot_entry, link); - free(snapshot_entry); - } return 0; } @@ -4956,6 +4951,7 @@ static void _spdk_bs_delete_open_cpl(void *cb_arg, struct spdk_blob *blob, int bserrno) { spdk_bs_sequence_t *seq = cb_arg; + struct spdk_blob_list *snapshot = NULL; uint32_t page_num; if (bserrno != 0) { @@ -4987,6 +4983,16 @@ _spdk_bs_delete_open_cpl(void *cb_arg, struct spdk_blob *blob, int bserrno) * get returned after this point by _spdk_blob_lookup(). */ TAILQ_REMOVE(&blob->bs->blobs, blob, link); + + /* If blob is a snapshot then remove it from the list */ + TAILQ_FOREACH(snapshot, &blob->bs->snapshots, link) { + if (snapshot->id == blob->id) { + TAILQ_REMOVE(&blob->bs->snapshots, snapshot, link); + free(snapshot); + break; + } + } + page_num = _spdk_bs_blobid_to_page(blob->id); spdk_bit_array_clear(blob->bs->used_blobids, page_num); blob->state = SPDK_BLOB_STATE_DIRTY; diff --git a/test/unit/lib/blob/blob.c/blob_ut.c b/test/unit/lib/blob/blob.c/blob_ut.c index 88f438eb3f..6d78572f0a 100644 --- a/test/unit/lib/blob/blob.c/blob_ut.c +++ b/test/unit/lib/blob/blob.c/blob_ut.c @@ -109,7 +109,18 @@ _get_xattr_value_null(void *arg, const char *name, *value = NULL; } +static int +_get_snapshots_count(struct spdk_blob_store *bs) +{ + struct spdk_blob_list *snapshot = NULL; + int count = 0; + TAILQ_FOREACH(snapshot, &bs->snapshots, link) { + count += 1; + } + + return count; +} static void bs_op_complete(void *cb_arg, int bserrno) @@ -524,6 +535,7 @@ blob_snapshot(void) struct spdk_blob_xattr_opts xattrs; spdk_blob_id blobid; spdk_blob_id snapshotid; + spdk_blob_id snapshotid2; const void *value; size_t value_len; int rc; @@ -551,9 +563,11 @@ blob_snapshot(void) CU_ASSERT(spdk_blob_get_num_clusters(blob) == 10) /* Create snapshot from blob */ + CU_ASSERT_EQUAL(_get_snapshots_count(bs), 0); spdk_bs_create_snapshot(bs, blobid, NULL, blob_op_with_id_complete, NULL); CU_ASSERT(g_bserrno == 0); CU_ASSERT(g_blobid != SPDK_BLOBID_INVALID); + CU_ASSERT_EQUAL(_get_snapshots_count(bs), 1); snapshotid = g_blobid; spdk_bs_open_blob(bs, snapshotid, blob_op_with_handle_complete, NULL); @@ -577,9 +591,10 @@ blob_snapshot(void) spdk_bs_create_snapshot(bs, blobid, &xattrs, blob_op_with_id_complete, NULL); CU_ASSERT(g_bserrno == 0); CU_ASSERT(g_blobid != SPDK_BLOBID_INVALID); - blobid = g_blobid; + CU_ASSERT_EQUAL(_get_snapshots_count(bs), 2); + snapshotid2 = g_blobid; - spdk_bs_open_blob(bs, blobid, blob_op_with_handle_complete, NULL); + spdk_bs_open_blob(bs, snapshotid2, blob_op_with_handle_complete, NULL); CU_ASSERT(g_bserrno == 0); SPDK_CU_ASSERT_FATAL(g_blob != NULL); snapshot2 = g_blob; @@ -620,16 +635,29 @@ blob_snapshot(void) spdk_bs_create_snapshot(bs, snapshotid, NULL, blob_op_with_id_complete, NULL); CU_ASSERT(g_bserrno == -EINVAL); CU_ASSERT(g_blobid == SPDK_BLOBID_INVALID); + CU_ASSERT_EQUAL(_get_snapshots_count(bs), 2); spdk_blob_close(blob, blob_op_complete, NULL); CU_ASSERT(g_bserrno == 0); - spdk_blob_close(snapshot, blob_op_complete, NULL); + spdk_bs_delete_blob(bs, blobid, blob_op_complete, NULL); CU_ASSERT(g_bserrno == 0); + CU_ASSERT_EQUAL(_get_snapshots_count(bs), 2); spdk_blob_close(snapshot2, blob_op_complete, NULL); CU_ASSERT(g_bserrno == 0); + spdk_bs_delete_blob(bs, snapshotid2, blob_op_complete, NULL); + CU_ASSERT(g_bserrno == 0); + CU_ASSERT_EQUAL(_get_snapshots_count(bs), 1); + + spdk_blob_close(snapshot, blob_op_complete, NULL); + CU_ASSERT(g_bserrno == 0); + + spdk_bs_delete_blob(bs, snapshotid, blob_op_complete, NULL); + CU_ASSERT(g_bserrno == 0); + CU_ASSERT_EQUAL(_get_snapshots_count(bs), 0); + spdk_bs_unload(g_bs, bs_op_complete, NULL); CU_ASSERT(g_bserrno == 0); g_bs = NULL;