cls/rbd: removing an in-use snapshot should move it to the trash

Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/cls/rbd/cls_rbd.cc b/src/cls/rbd/cls_rbd.cc
index 97ce99a9fee533d0b60c7af4bcc998810b53a43d..4a08a6f91eb735fefbfa3ba13a70fb849b75cd7c 100644 (file)
--- a/src/cls/rbd/cls_rbd.cc
+++ b/src/cls/rbd/cls_rbd.cc
@@ -31,6 +31,7 @@
 #include <errno.h>
 #include <sstream>
 
+#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 fa19a5b662639404a24c2a230549bb7d8d8b9564..67cc5f8e1d16559397c77b23287d872a9aba5e0b 100644 (file)
--- 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 6225f055e61ce82183d1b4c0fb80cc89a2ba283f..4d5ba7c5792c19840f82c68dd1d3ad56df311088 100644 (file)
--- 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 0bd8d754bdf3f6d35f0aa7b3326e6de4a439397b..a915479b68dbf90e11156ebc989a96289ae18663 100644 (file)
--- 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)