From b98727ecbe0a96712dc5b10c7e771bf4c218583a Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Fri, 19 Jan 2018 08:15:06 -0500 Subject: [PATCH] cls/rbd: removing an in-use snapshot should move it to the trash Signed-off-by: Jason Dillaman --- src/cls/rbd/cls_rbd.cc | 140 ++++++++++++++++++++++++++++--- src/cls/rbd/cls_rbd_client.cc | 8 ++ src/cls/rbd/cls_rbd_client.h | 3 + src/test/cls_rbd/test_cls_rbd.cc | 39 +++++++-- 4 files changed, 169 insertions(+), 21 deletions(-) diff --git a/src/cls/rbd/cls_rbd.cc b/src/cls/rbd/cls_rbd.cc index 97ce99a9fee53..4a08a6f91eb73 100644 --- a/src/cls/rbd/cls_rbd.cc +++ b/src/cls/rbd/cls_rbd.cc @@ -31,6 +31,7 @@ #include #include +#include "include/uuid.h" #include "common/bit_vector.hpp" #include "common/errno.h" #include "objclass/objclass.h" @@ -277,6 +278,27 @@ int snapshot_iterate(cls_method_context_t hctx, L& lambda) { return 0; } +int snapshot_trash_add(cls_method_context_t hctx, + const std::string& snapshot_key, cls_rbd_snap snap) { + // add snap_trash feature bit if not already enabled + int r = image::set_op_features(hctx, RBD_OPERATION_FEATURE_SNAP_TRASH, + RBD_OPERATION_FEATURE_SNAP_TRASH); + if (r < 0) { + return r; + } + + snap.snapshot_namespace = cls::rbd::TrashSnapshotNamespace{snap.name}; + uuid_d uuid_gen; + uuid_gen.generate_random(); + snap.name = uuid_gen.to_string(); + + r = write_key(hctx, snapshot_key, snap); + if (r < 0) { + return r; + } + return 0; +} + } // namespace image /** @@ -1847,6 +1869,16 @@ int snapshot_add(cls_method_context_t hctx, bufferlist *in, bufferlist *out) return r; } + if (cls::rbd::get_snap_namespace_type(snap_meta.snapshot_namespace) == + cls::rbd::SNAPSHOT_NAMESPACE_TYPE_TRASH) { + // add snap_trash feature bit if not already enabled + r = image::set_op_features(hctx, RBD_OPERATION_FEATURE_SNAP_TRASH, + RBD_OPERATION_FEATURE_SNAP_TRASH); + if (r < 0) { + return r; + } + } + return 0; } @@ -1952,34 +1984,68 @@ int snapshot_remove(cls_method_context_t hctx, bufferlist *in, bufferlist *out) string snapshot_key; key_from_snap_id(snap_id, &snapshot_key); int r = read_key(hctx, snapshot_key, &snap); - if (r == -ENOENT) + if (r == -ENOENT) { return -ENOENT; + } - if (snap.protection_status != RBD_PROTECTION_STATUS_UNPROTECTED) + if (snap.protection_status != RBD_PROTECTION_STATUS_UNPROTECTED) { return -EBUSY; + } - auto remove_lambda = [snap_id](const cls_rbd_snap& snap_meta) { - if (snap_meta.id != snap_id && snap_meta.parent.pool != -1) { - return -EEXIST; + // snapshot is in-use by clone v2 child + if (snap.child_count > 0) { + if (cls::rbd::get_snap_namespace_type(snap.snapshot_namespace) == + cls::rbd::SNAPSHOT_NAMESPACE_TYPE_TRASH) { + // trash snapshot still in-use + return -EBUSY; + } + + r = image::snapshot_trash_add(hctx, snapshot_key, snap); + if (r < 0) { + return r; } + return 0; - }; + } - r = image::snapshot_iterate(hctx, remove_lambda); - bool has_child_snaps = (r == -EEXIST); - if (r < 0 && r != -EEXIST) { + r = remove_key(hctx, snapshot_key); + if (r < 0) { return r; } - r = cls_cxx_map_remove_key(hctx, snapshot_key); + bool has_child_snaps = false; + bool has_trash_snaps = false; + auto remove_lambda = + [snap_id, &has_child_snaps, &has_trash_snaps](const cls_rbd_snap& snap_meta) { + if (snap_meta.id != snap_id) { + if (snap_meta.parent.pool != -1) { + has_child_snaps = true; + } + if (cls::rbd::get_snap_namespace_type(snap_meta.snapshot_namespace) == + cls::rbd::SNAPSHOT_NAMESPACE_TYPE_TRASH) { + has_trash_snaps = true; + } + } + return 0; + }; + + r = image::snapshot_iterate(hctx, remove_lambda); if (r < 0) { - CLS_ERR("error writing snapshot metadata: %s", cpp_strerror(r).c_str()); return r; } + uint64_t op_features_mask = 0ULL; if (!has_child_snaps) { // disable clone child op feature if no longer associated - r = image::set_op_features(hctx, 0, RBD_OPERATION_FEATURE_CLONE_CHILD); + op_features_mask |= RBD_OPERATION_FEATURE_CLONE_CHILD; + } + if (!has_trash_snaps) { + // remove the snap_trash op feature if not in-use by any other snapshots + op_features_mask |= RBD_OPERATION_FEATURE_SNAP_TRASH; + } + + if (op_features_mask != 0ULL) { + r = image::set_op_features(hctx, 0, op_features_mask); if (r < 0) { return r; } @@ -1988,6 +2054,52 @@ int snapshot_remove(cls_method_context_t hctx, bufferlist *in, bufferlist *out) return 0; } +/** + * Moves a snapshot to the trash namespace. + * + * Input: + * @param snap_id the id of the snapshot to move to the trash (uint64_t) + * + * Output: + * @returns 0 on success, negative error code on failure + */ +int snapshot_trash_add(cls_method_context_t hctx, bufferlist *in, + bufferlist *out) +{ + snapid_t snap_id; + + try { + bufferlist::iterator iter = in->begin(); + decode(snap_id, iter); + } catch (const buffer::error &err) { + return -EINVAL; + } + + CLS_LOG(20, "snapshot_trash_add id=%" PRIu64, snap_id.val); + + cls_rbd_snap snap; + std::string snapshot_key; + key_from_snap_id(snap_id, &snapshot_key); + int r = read_key(hctx, snapshot_key, &snap); + if (r == -ENOENT) { + return r; + } + + if (snap.protection_status != RBD_PROTECTION_STATUS_UNPROTECTED) { + return -EBUSY; + } else if (cls::rbd::get_snap_namespace_type(snap.snapshot_namespace) == + cls::rbd::SNAPSHOT_NAMESPACE_TYPE_TRASH) { + return -EINVAL; + } + + r = image::snapshot_trash_add(hctx, snapshot_key, snap); + if (r < 0) { + return r; + } + + return 0; +} + /** * Returns a uint64_t of all the features supported by this class. */ @@ -5953,6 +6065,7 @@ CLS_INIT(rbd) cls_method_handle_t h_snapshot_add; cls_method_handle_t h_snapshot_remove; cls_method_handle_t h_snapshot_rename; + cls_method_handle_t h_snapshot_trash_add; cls_method_handle_t h_get_all_features; cls_method_handle_t h_copyup; cls_method_handle_t h_get_id; @@ -6068,6 +6181,9 @@ CLS_INIT(rbd) cls_register_cxx_method(h_class, "snapshot_rename", CLS_METHOD_RD | CLS_METHOD_WR, snapshot_rename, &h_snapshot_rename); + cls_register_cxx_method(h_class, "snapshot_trash_add", + CLS_METHOD_RD | CLS_METHOD_WR, + snapshot_trash_add, &h_snapshot_trash_add); cls_register_cxx_method(h_class, "get_all_features", CLS_METHOD_RD, get_all_features, &h_get_all_features); diff --git a/src/cls/rbd/cls_rbd_client.cc b/src/cls/rbd/cls_rbd_client.cc index fa19a5b662639..67cc5f8e1d165 100644 --- a/src/cls/rbd/cls_rbd_client.cc +++ b/src/cls/rbd/cls_rbd_client.cc @@ -641,6 +641,14 @@ namespace librbd { op->exec("rbd", "snapshot_rename", bl); } + void snapshot_trash_add(librados::ObjectWriteOperation *op, + snapid_t snap_id) + { + bufferlist bl; + encode(snap_id, bl); + op->exec("rbd", "snapshot_trash_add", bl); + } + void get_snapcontext_start(librados::ObjectReadOperation *op) { bufferlist bl; diff --git a/src/cls/rbd/cls_rbd_client.h b/src/cls/rbd/cls_rbd_client.h index 6225f055e61ce..4d5ba7c5792c1 100644 --- a/src/cls/rbd/cls_rbd_client.h +++ b/src/cls/rbd/cls_rbd_client.h @@ -139,6 +139,9 @@ namespace librbd { void snapshot_rename(librados::ObjectWriteOperation *op, snapid_t src_snap_id, const std::string &dst_name); + void snapshot_trash_add(librados::ObjectWriteOperation *op, + snapid_t snap_id); + void get_snapcontext_start(librados::ObjectReadOperation *op); int get_snapcontext_finish(bufferlist::iterator *it, ::SnapContext *snapc); diff --git a/src/test/cls_rbd/test_cls_rbd.cc b/src/test/cls_rbd/test_cls_rbd.cc index 0bd8d754bdf3f..a915479b68dbf 100644 --- a/src/test/cls_rbd/test_cls_rbd.cc +++ b/src/test/cls_rbd/test_cls_rbd.cc @@ -2530,15 +2530,18 @@ TEST_F(TestClsRbd, clone_parent) // op feature should have been enabled uint64_t op_features; + uint64_t expected_op_features = RBD_OPERATION_FEATURE_CLONE_PARENT; ASSERT_EQ(0, op_features_get(&ioctx, oid, &op_features)); - ASSERT_TRUE((op_features & RBD_OPERATION_FEATURE_CLONE_PARENT) == - RBD_OPERATION_FEATURE_CLONE_PARENT); + ASSERT_TRUE((op_features & expected_op_features) == expected_op_features); // cannot attach to trashed snapshot - librados::ObjectWriteOperation op; - ::librbd::cls_client::snapshot_add(&op, 234, "trash_snap", - cls::rbd::TrashSnapshotNamespace()); - ASSERT_EQ(0, ioctx.operate(oid, &op)); + librados::ObjectWriteOperation op1; + ::librbd::cls_client::snapshot_add(&op1, 234, "trash_snap", + cls::rbd::UserSnapshotNamespace()); + ASSERT_EQ(0, ioctx.operate(oid, &op1)); + librados::ObjectWriteOperation op2; + ::librbd::cls_client::snapshot_trash_add(&op2, 234); + ASSERT_EQ(0, ioctx.operate(oid, &op2)); ASSERT_EQ(-ENOENT, child_attach(&ioctx, oid, 234, {})); cls::rbd::ChildImageSpecs child_images; @@ -2548,6 +2551,19 @@ TEST_F(TestClsRbd, clone_parent) {1, "image1"}, {1, "image2"}, {2, "image2"}}; ASSERT_EQ(expected_child_images, child_images); + // move snapshot to the trash + ASSERT_EQ(0, snapshot_remove(&ioctx, oid, 123)); + ASSERT_EQ(-EBUSY, snapshot_remove(&ioctx, oid, 123)); + ASSERT_EQ(0, snapshot_get(&ioctx, oid, {123}, &snaps, + &parents, &protection_status)); + ASSERT_EQ(1U, snaps.size()); + ASSERT_EQ(cls::rbd::SNAPSHOT_NAMESPACE_TYPE_TRASH, + cls::rbd::get_snap_namespace_type(snaps[0].snapshot_namespace)); + + expected_op_features |= RBD_OPERATION_FEATURE_SNAP_TRASH; + ASSERT_EQ(0, op_features_get(&ioctx, oid, &op_features)); + ASSERT_TRUE((op_features & expected_op_features) == expected_op_features); + expected_child_images = {{1, "image1"}, {2, "image2"}}; ASSERT_EQ(0, child_detach(&ioctx, oid, 123, {1, "image2"})); ASSERT_EQ(0, children_list(&ioctx, oid, 123, &child_images)); @@ -2556,14 +2572,19 @@ TEST_F(TestClsRbd, clone_parent) ASSERT_EQ(0, child_detach(&ioctx, oid, 123, {2, "image2"})); ASSERT_EQ(0, op_features_get(&ioctx, oid, &op_features)); - ASSERT_TRUE((op_features & RBD_OPERATION_FEATURE_CLONE_PARENT) == - RBD_OPERATION_FEATURE_CLONE_PARENT); + ASSERT_TRUE((op_features & expected_op_features) == expected_op_features); ASSERT_EQ(0, child_detach(&ioctx, oid, 123, {1, "image1"})); ASSERT_EQ(-ENOENT, children_list(&ioctx, oid, 123, &child_images)); + ASSERT_EQ(0, snapshot_remove(&ioctx, oid, 234)); + ASSERT_EQ(0, op_features_get(&ioctx, oid, &op_features)); + ASSERT_TRUE((op_features & expected_op_features) == + RBD_OPERATION_FEATURE_SNAP_TRASH); + + ASSERT_EQ(0, snapshot_remove(&ioctx, oid, 123)); ASSERT_EQ(0, op_features_get(&ioctx, oid, &op_features)); - ASSERT_TRUE((op_features & RBD_OPERATION_FEATURE_CLONE_PARENT) == 0ULL); + ASSERT_TRUE((op_features & expected_op_features) == 0); } TEST_F(TestClsRbd, clone_child) -- 2.39.5