ivampiresp/Spdk
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

lib/blob: Fix deleting a snapshot after decoupling it from its parent

Browse Source 
When decoupling a snapshot from its parent, we need to clear its parent.
So we should remove the xattr BLOB_SNAPSHOT. Modifying the xattrs of a blob
only works if its metadata are not in read-only mode.
By default, a snapshot is in read-only mode so this operation fails. When we
later want to delete the snapshot, we will see that it has a parent, so we will
try to remove the snapshot from its parent's clones list. This will cause a
crash.
The fix is to remove the BLOB_SNAPSHOT xattr only after setting the snapshot's
metadata in rw mode.

Signed-off-by: Alex Michon <amichon@kalrayinc.com>
Change-Id: I80efa6dd3dcb38b4c738ce2e97aa2ffc281cefa5
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/13723
Community-CI: Mellanox Build Bot
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@nvidia.com>
This commit is contained in:
vagrant 2022-07-20 09:37:02 +00:00 committed by Tomasz Zawadzki
parent 5de98ef86c
commit fa09c9ac9b
2 changed files with 60 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
lib/blob/blobstore.c
View File

@ -6333,7 +6333,6 @@ bs_inflate_blob_done(struct spdk_clone_snapshot_ctx *ctx)
if (ctx->allocate_all) {
/* remove thin provisioning */
bs_blob_list_remove(_blob);
blob_remove_xattr(_blob, BLOB_SNAPSHOT, true);
_blob->invalid_flags = _blob->invalid_flags & ~SPDK_BLOB_THIN_PROV;
_blob->back_bs_dev->destroy(_blob->back_bs_dev);
_blob->back_bs_dev = NULL;
@ -6348,7 +6347,6 @@ bs_inflate_blob_done(struct spdk_clone_snapshot_ctx *ctx)
}
bs_blob_list_remove(_blob);
blob_remove_xattr(_blob, BLOB_SNAPSHOT, true);
_blob->parent_id = SPDK_BLOBID_INVALID;
_blob->back_bs_dev->destroy(_blob->back_bs_dev);
_blob->back_bs_dev = bs_create_zeroes_dev();
@ -6356,6 +6354,7 @@ bs_inflate_blob_done(struct spdk_clone_snapshot_ctx *ctx)
/* Temporarily override md_ro flag for MD modification */
_blob->md_ro = false;
blob_remove_xattr(_blob, BLOB_SNAPSHOT, true);
_blob->state = SPDK_BLOB_STATE_DIRTY;
spdk_blob_sync_md(_blob, bs_clone_snapshot_origblob_cleanup, ctx);

110
test/unit/lib/blob/blob.c/blob_ut.c
View File

@ -7246,66 +7246,74 @@ blob_decouple_snapshot(void)
spdk_blob_id blobid, snapshotid;
uint64_t cluster;
channel = spdk_bs_alloc_io_channel(bs);
SPDK_CU_ASSERT_FATAL(channel != NULL);
for (int delete_snapshot_first = 0; delete_snapshot_first <= 1; delete_snapshot_first++) {
channel = spdk_bs_alloc_io_channel(bs);
SPDK_CU_ASSERT_FATAL(channel != NULL);
ut_spdk_blob_opts_init(&opts);
opts.num_clusters = 10;
opts.thin_provision = false;
ut_spdk_blob_opts_init(&opts);
opts.num_clusters = 10;
opts.thin_provision = false;
blob = ut_blob_create_and_open(bs, &opts);
blobid = spdk_blob_get_id(blob);
blob = ut_blob_create_and_open(bs, &opts);
blobid = spdk_blob_get_id(blob);
/* Create first 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;
/* Create first 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;
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);
snapshot1 = g_blob;
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);
snapshot1 = g_blob;
/* Create the second one */
CU_ASSERT_EQUAL(_get_snapshots_count(bs), 1);
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), 2);
snapshotid = g_blobid;
/* Create the second one */
CU_ASSERT_EQUAL(_get_snapshots_count(bs), 1);
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), 2);
snapshotid = g_blobid;
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);
snapshot2 = g_blob;
CU_ASSERT_EQUAL(spdk_blob_get_parent_snapshot(bs, snapshot2->id), snapshot1->id);
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);
snapshot2 = g_blob;
CU_ASSERT_EQUAL(spdk_blob_get_parent_snapshot(bs, snapshot2->id), snapshot1->id);
/* Now decouple the second snapshot forcing it to copy the written clusters */
spdk_bs_blob_decouple_parent(bs, channel, snapshot2->id, blob_op_complete, NULL);
poll_threads();
CU_ASSERT(g_bserrno == 0);
/* Now decouple the second snapshot forcing it to copy the written clusters */
spdk_bs_blob_decouple_parent(bs, channel, snapshot2->id, blob_op_complete, NULL);
poll_threads();
CU_ASSERT(g_bserrno == 0);
/* Verify that the snapshot has been decoupled and that the clusters have been copied */
CU_ASSERT_EQUAL(spdk_blob_get_parent_snapshot(bs, snapshot2->id), SPDK_BLOBID_INVALID);
for (cluster = 0; cluster < snapshot2->active.num_clusters; ++cluster) {
CU_ASSERT_NOT_EQUAL(snapshot2->active.clusters[cluster], 0);
CU_ASSERT_NOT_EQUAL(snapshot2->active.clusters[cluster],
snapshot1->active.clusters[cluster]);
/* Verify that the snapshot has been decoupled and that the clusters have been copied */
CU_ASSERT_EQUAL(spdk_blob_get_parent_snapshot(bs, snapshot2->id), SPDK_BLOBID_INVALID);
for (cluster = 0; cluster < snapshot2->active.num_clusters; ++cluster) {
CU_ASSERT_NOT_EQUAL(snapshot2->active.clusters[cluster], 0);
CU_ASSERT_NOT_EQUAL(snapshot2->active.clusters[cluster],
snapshot1->active.clusters[cluster]);
}
spdk_bs_free_io_channel(channel);
if (delete_snapshot_first) {
ut_blob_close_and_delete(bs, snapshot2);
ut_blob_close_and_delete(bs, snapshot1);
ut_blob_close_and_delete(bs, blob);
} else {
ut_blob_close_and_delete(bs, blob);
ut_blob_close_and_delete(bs, snapshot2);
ut_blob_close_and_delete(bs, snapshot1);
}
poll_threads();
}
spdk_bs_free_io_channel(channel);
ut_blob_close_and_delete(bs, snapshot2);
ut_blob_close_and_delete(bs, snapshot1);
ut_blob_close_and_delete(bs, blob);
poll_threads();
}
static void