cls/rbd: tweak handling for moving snapshots to trash namespace

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 4a08a6f91eb735fefbfa3ba13a70fb849b75cd7c..777cdbb524f01847dba48dabf695d0d85eb77599 100644 (file)
--- a/src/cls/rbd/cls_rbd.cc
+++ b/src/cls/rbd/cls_rbd.cc
@@ -278,27 +278,6 @@ 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
 
 /**
@@ -1994,18 +1973,7 @@ int snapshot_remove(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
 
   // 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;
+    return -EBUSY;
   }
 
   r = remove_key(hctx, snapshot_key);
@@ -2089,10 +2057,22 @@ int snapshot_trash_add(cls_method_context_t hctx, bufferlist *in,
     return -EBUSY;
   } else if (cls::rbd::get_snap_namespace_type(snap.snapshot_namespace) ==
                cls::rbd::SNAPSHOT_NAMESPACE_TYPE_TRASH) {
-    return -EINVAL;
+    return -EEXIST;
   }
 
-  r = image::snapshot_trash_add(hctx, snapshot_key, snap);
+  // 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;
+  }
+
+  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;
   }

diff --git a/src/librbd/operation/SnapshotRemoveRequest.cc b/src/librbd/operation/SnapshotRemoveRequest.cc
index 4622edfd05991b7c1e4e79f4cf63acc5fc8adbbb..8e873e1461783d806c5352ad9805ecd53455a8c7 100644 (file)
--- a/src/librbd/operation/SnapshotRemoveRequest.cc
+++ b/src/librbd/operation/SnapshotRemoveRequest.cc
@@ -97,7 +97,7 @@ void SnapshotRemoveRequest<I>::handle_trash_snap(int r) {
     // trash / clone v2 not supported
     detach_child();
     return;
-  } else if (r < 0) {
+  } else if (r < 0 && r != -EEXIST) {
     lderr(cct) << "failed to move snapshot to trash: " << cpp_strerror(r)
                << dendl;
     this->complete(r);

diff --git a/src/test/cls_rbd/test_cls_rbd.cc b/src/test/cls_rbd/test_cls_rbd.cc
index a915479b68dbf90e11156ebc989a96289ae18663..3f6383a7cfbf41b6f4ce6e635480cb50885c968f 100644 (file)
--- a/src/test/cls_rbd/test_cls_rbd.cc
+++ b/src/test/cls_rbd/test_cls_rbd.cc
@@ -2552,8 +2552,10 @@ TEST_F(TestClsRbd, clone_parent)
   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));
+  librados::ObjectWriteOperation op3;
+  ::librbd::cls_client::snapshot_trash_add(&op3, 123);
+  ASSERT_EQ(0, ioctx.operate(oid, &op3));
   ASSERT_EQ(0, snapshot_get(&ioctx, oid, {123}, &snaps,
                             &parents, &protection_status));
   ASSERT_EQ(1U, snaps.size());