From 6e1434eefc3dd4c815359a85d9d51a798983d9a2 Mon Sep 17 00:00:00 2001
From: Mykola Golub <mgolub@suse.com>
Date: Mon, 15 Apr 2019 11:35:53 +0100
Subject: [PATCH] librbd: optionally move parent image to trash on remove

and auto-delete when the last clone is detached

Signed-off-by: Mykola Golub <mgolub@suse.com>
---
 qa/workunits/rbd/cli_generic.sh               | 15 +++
 src/cls/rbd/cls_rbd_types.h                   |  1 +
 src/common/options.cc                         |  4 +
 src/include/rbd/librbd.h                      |  3 +-
 src/librbd/api/Image.cc                       | 10 +-
 src/librbd/image/DetachChildRequest.cc        | 88 ++++++++++++++++-
 src/librbd/image/DetachChildRequest.h         | 25 +++--
 src/librbd/image/PreRemoveRequest.cc          | 19 ++--
 src/librbd/image/PreRemoveRequest.h           |  1 +
 src/librbd/image/RemoveRequest.cc             |  3 +
 src/librbd/operation/SnapshotRemoveRequest.cc |  2 +-
 .../image/test_mock_DetachChildRequest.cc     | 98 +++++++++++++++++++
 src/tools/rbd/action/Trash.cc                 |  6 +-
 13 files changed, 256 insertions(+), 19 deletions(-)

diff --git a/qa/workunits/rbd/cli_generic.sh b/qa/workunits/rbd/cli_generic.sh
index 1a46df10c5103..b4b8f4774c850 100755
--- a/qa/workunits/rbd/cli_generic.sh
+++ b/qa/workunits/rbd/cli_generic.sh
@@ -576,6 +576,21 @@ test_clone_v2() {
     rbd snap list --all test1 | wc -l | grep '^0$'
     rbd rm test1
     rbd rm test2
+
+    rbd create $RBD_CREATE_ARGS -s 1 test1
+    rbd snap create test1@1
+    rbd snap create test1@2
+    rbd clone test1@1 test2 --rbd-default-clone-format 2
+    rbd clone test1@2 test3 --rbd-default-clone-format 2
+    rbd snap rm test1@1
+    rbd snap rm test1@2
+    expect_fail rbd rm test1
+    rbd rm test1 --rbd-move-parent-to-trash-on-remove=true
+    rbd trash ls -a | grep test1
+    rbd rm test2
+    rbd trash ls -a | grep test1
+    rbd rm test3
+    rbd trash ls -a | expect_fail grep test1
 }
 
 test_thick_provision() {
diff --git a/src/cls/rbd/cls_rbd_types.h b/src/cls/rbd/cls_rbd_types.h
index bbfc85870b21b..74dfe1d99de20 100644
--- a/src/cls/rbd/cls_rbd_types.h
+++ b/src/cls/rbd/cls_rbd_types.h
@@ -560,6 +560,7 @@ enum TrashImageSource {
   TRASH_IMAGE_SOURCE_MIRRORING = 1,
   TRASH_IMAGE_SOURCE_MIGRATION = 2,
   TRASH_IMAGE_SOURCE_REMOVING = 3,
+  TRASH_IMAGE_SOURCE_USER_PARENT= 4,
 };
 
 inline std::ostream& operator<<(std::ostream& os,
diff --git a/src/common/options.cc b/src/common/options.cc
index f7db2a72aaa05..34cf35ca5356d 100644
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -7153,6 +7153,10 @@ static std::vector<Option> get_rbd_options() {
     .set_default(0)
     .set_description("default number of seconds to protect deleted images in the trash"),
 
+    Option("rbd_move_parent_to_trash_on_remove", Option::TYPE_BOOL, Option::LEVEL_BASIC)
+    .set_default(false)
+    .set_description("move parent with clone format v2 children to the trash when deleted"),
+
     Option("rbd_mirroring_resync_after_disconnect", Option::TYPE_BOOL, Option::LEVEL_ADVANCED)
     .set_default(false)
     .set_description("automatically start image resync after mirroring is disconnected due to being laggy"),
diff --git a/src/include/rbd/librbd.h b/src/include/rbd/librbd.h
index 1adffe781cf68..1fa30dda750d8 100644
--- a/src/include/rbd/librbd.h
+++ b/src/include/rbd/librbd.h
@@ -238,7 +238,8 @@ typedef enum {
   RBD_TRASH_IMAGE_SOURCE_USER = 0,
   RBD_TRASH_IMAGE_SOURCE_MIRRORING = 1,
   RBD_TRASH_IMAGE_SOURCE_MIGRATION = 2,
-  RBD_TRASH_IMAGE_SOURCE_REMOVING = 3
+  RBD_TRASH_IMAGE_SOURCE_REMOVING = 3,
+  RBD_TRASH_IMAGE_SOURCE_USER_PARENT = 4,
 } rbd_trash_image_source_t;
 
 typedef struct {
diff --git a/src/librbd/api/Image.cc b/src/librbd/api/Image.cc
index 13c5a90d8c547..8fbd7176652ce 100644
--- a/src/librbd/api/Image.cc
+++ b/src/librbd/api/Image.cc
@@ -767,7 +767,15 @@ int Image<I>::remove(IoCtx& io_ctx, const std::string &image_name,
       // attempt to pre-validate the removal before moving to trash and
       // removing
       r = pre_remove_image<I>(io_ctx, image_id);
-      if (r < 0 && r != -ENOENT) {
+      if (r == -ECHILD) {
+        if (config.get_val<bool>("rbd_move_parent_to_trash_on_remove")) {
+          // keep the image in the trash until the last child is removed
+          trash_image_source = RBD_TRASH_IMAGE_SOURCE_USER_PARENT;
+        } else {
+          lderr(cct) << "image has snapshots - not removing" << dendl;
+          return -ENOTEMPTY;
+        }
+      } else if (r < 0 && r != -ENOENT) {
         return r;
       }
     }
diff --git a/src/librbd/image/DetachChildRequest.cc b/src/librbd/image/DetachChildRequest.cc
index 89f5c949a9053..39221e1b25dc8 100644
--- a/src/librbd/image/DetachChildRequest.cc
+++ b/src/librbd/image/DetachChildRequest.cc
@@ -10,6 +10,7 @@
 #include "librbd/ImageState.h"
 #include "librbd/Operations.h"
 #include "librbd/Utils.h"
+#include "librbd/trash/RemoveRequest.h"
 #include <string>
 
 #define dout_subsys ceph_subsys_rbd
@@ -205,9 +206,94 @@ void DetachChildRequest<I>::handle_clone_v2_remove_snapshot(int r) {
   if (r < 0 && r != -ENOENT) {
     ldout(cct, 5) << "failed to remove trashed clone snapshot: "
                   << cpp_strerror(r) << dendl;
+    clone_v2_close_parent();
+    return;
+  }
+
+  if (m_parent_image_ctx->snaps.empty()) {
+    clone_v2_get_parent_trash_entry();
+  } else {
+    clone_v2_close_parent();
+  }
+}
+
+template<typename I>
+void DetachChildRequest<I>::clone_v2_get_parent_trash_entry() {
+  auto cct = m_image_ctx.cct;
+  ldout(cct, 5) << dendl;
+
+  librados::ObjectReadOperation op;
+  cls_client::trash_get_start(&op, m_parent_image_ctx->id);
+
+  m_out_bl.clear();
+  auto aio_comp = create_rados_callback<
+    DetachChildRequest<I>,
+    &DetachChildRequest<I>::handle_clone_v2_get_parent_trash_entry>(this);
+  int r = m_parent_io_ctx.aio_operate(RBD_TRASH, aio_comp, &op, &m_out_bl);
+  ceph_assert(r == 0);
+  aio_comp->release();
+}
+
+template<typename I>
+void DetachChildRequest<I>::handle_clone_v2_get_parent_trash_entry(int r) {
+  auto cct = m_image_ctx.cct;
+  ldout(cct, 5) << "r=" << r << dendl;
+
+  if (r < 0 && r != -ENOENT) {
+    ldout(cct, 5) << "failed to get parent trash entry: " << cpp_strerror(r)
+                  << dendl;
+    clone_v2_close_parent();
+    return;
+  }
+
+  bool in_trash = false;
+
+  if (r == 0) {
+    cls::rbd::TrashImageSpec trash_spec;
+    auto it = m_out_bl.cbegin();
+    r = cls_client::trash_get_finish(&it, &trash_spec);
+
+    if (r == 0 &&
+        trash_spec.source == cls::rbd::TRASH_IMAGE_SOURCE_USER_PARENT &&
+        trash_spec.state == cls::rbd::TRASH_IMAGE_STATE_NORMAL &&
+        trash_spec.deferment_end_time <= ceph_clock_now()) {
+      in_trash = true;
+    }
+  }
+
+  if (in_trash) {
+    clone_v2_remove_parent_from_trash();
+  } else {
+    clone_v2_close_parent();
   }
+}
 
-  clone_v2_close_parent();
+template<typename I>
+void DetachChildRequest<I>::clone_v2_remove_parent_from_trash() {
+  auto cct = m_image_ctx.cct;
+  ldout(cct, 5) << dendl;
+
+  auto ctx = create_context_callback<
+    DetachChildRequest<I>,
+    &DetachChildRequest<I>::handle_clone_v2_remove_parent_from_trash>(this);
+  auto req = librbd::trash::RemoveRequest<I>::create(
+      m_parent_io_ctx, m_parent_image_ctx, m_image_ctx.op_work_queue, false,
+      m_no_op, ctx);
+  req->send();
+}
+
+template<typename I>
+void DetachChildRequest<I>::handle_clone_v2_remove_parent_from_trash(int r) {
+  auto cct = m_image_ctx.cct;
+  ldout(cct, 5) << "r=" << r << dendl;
+
+  if (r < 0) {
+    ldout(cct, 5) << "failed to remove parent image:" << cpp_strerror(r)
+                  << dendl;
+  }
+
+  m_parent_image_ctx = nullptr;
+  finish(0);
 }
 
 template<typename I>
diff --git a/src/librbd/image/DetachChildRequest.h b/src/librbd/image/DetachChildRequest.h
index bb0c81d21af85..646b7ec629d6e 100644
--- a/src/librbd/image/DetachChildRequest.h
+++ b/src/librbd/image/DetachChildRequest.h
@@ -8,6 +8,7 @@
 #include "include/buffer.h"
 #include "include/rados/librados.hpp"
 #include "librbd/Types.h"
+#include "librbd/internal.h"
 
 class Context;
 
@@ -50,13 +51,16 @@ private:
    *    |                               v
    *    |                           OPEN_PARENT
    *    |                               |
-   *    |                               v
-   *    |                           REMOVE_SNAPSHOT
-   *    |                               |
-   *    |                               v
-   *    |                           CLOSE_PARENT
-   *    |                               |
-   *    |/------------------------------/
+   *    |                               v           (has more children)
+   *    |                           REMOVE_SNAPSHOT ---------------\
+   *    |                               |                          |
+   *    |                               v                  (noent) |
+   *    |     (auto-delete when     GET_PARENT_TRASH_ENTRY . . . .\|
+   *    |      last child detached)     |                          |
+   *    |                               v                          v
+   *    |                           REMOVE_PARENT_FROM_TRASH   CLOSE_PARENT
+   *    |                               |                          |
+   *    |/------------------------------/--------------------------/
    *    |
    *    v
    * <finish>
@@ -77,6 +81,7 @@ private:
   ImageCtxT* m_parent_image_ctx = nullptr;
 
   ceph::bufferlist m_out_bl;
+  NoOpProgressContext m_no_op;
 
   void clone_v2_child_detach();
   void handle_clone_v2_child_detach(int r);
@@ -90,6 +95,12 @@ private:
   void clone_v2_remove_snapshot();
   void handle_clone_v2_remove_snapshot(int r);
 
+  void clone_v2_get_parent_trash_entry();
+  void handle_clone_v2_get_parent_trash_entry(int r);
+
+  void clone_v2_remove_parent_from_trash();
+  void handle_clone_v2_remove_parent_from_trash(int r);
+
   void clone_v2_close_parent();
   void handle_clone_v2_close_parent(int r);
 
diff --git a/src/librbd/image/PreRemoveRequest.cc b/src/librbd/image/PreRemoveRequest.cc
index 0ed6cf946adc9..48e89e5b5f2df 100644
--- a/src/librbd/image/PreRemoveRequest.cc
+++ b/src/librbd/image/PreRemoveRequest.cc
@@ -143,7 +143,7 @@ void PreRemoveRequest<I>::check_image_snaps() {
     } else {
       m_image_ctx->snap_lock.put_read();
 
-      lderr(cct) << "image has snapshots - not removing" << dendl;
+      ldout(cct, 5) << "image has snapshots - not removing" << dendl;
       finish(-ENOTEMPTY);
       return;
     }
@@ -272,14 +272,15 @@ void PreRemoveRequest<I>::handle_remove_snapshot(int r) {
   auto cct = m_image_ctx->cct;
   ldout(cct, 5) << "r=" << r << dendl;
 
-  if (r < 0 && r != -ENOENT) {
+  if (r == -EBUSY) {
+    ldout(cct, 5) << "skipping attached child" << dendl;
+    if (m_ret_val == 0) {
+      m_ret_val = -ECHILD;
+    }
+  } else if (r < 0 && r != -ENOENT) {
     auto snap_id = m_snap_infos.begin()->first;
     lderr(cct) << "failed to auto-prune snapshot " << snap_id << ": "
                << cpp_strerror(r) << dendl;
-
-    if (r == -EBUSY) {
-      r = -ENOTEMPTY;
-    }
     finish(r);
     return;
   }
@@ -295,7 +296,11 @@ void PreRemoveRequest<I>::finish(int r) {
   auto cct = m_image_ctx->cct;
   ldout(cct, 5) << "r=" << r << dendl;
 
-  m_on_finish->complete(r);
+  if (m_ret_val == 0) {
+    m_ret_val = r;
+  }
+
+  m_on_finish->complete(m_ret_val);
   delete this;
 }
 
diff --git a/src/librbd/image/PreRemoveRequest.h b/src/librbd/image/PreRemoveRequest.h
index 99981491b651d..3bb6ca6434d97 100644
--- a/src/librbd/image/PreRemoveRequest.h
+++ b/src/librbd/image/PreRemoveRequest.h
@@ -69,6 +69,7 @@ private:
   std::list<obj_watch_t> m_watchers;
 
   std::map<uint64_t, SnapInfo> m_snap_infos;
+  int m_ret_val = 0;
 
   void acquire_exclusive_lock();
   void handle_exclusive_lock(int r);
diff --git a/src/librbd/image/RemoveRequest.cc b/src/librbd/image/RemoveRequest.cc
index 88af466615a5e..d5154b977ee58 100644
--- a/src/librbd/image/RemoveRequest.cc
+++ b/src/librbd/image/RemoveRequest.cc
@@ -122,6 +122,9 @@ void RemoveRequest<I>::handle_pre_remove_image(int r) {
   ldout(m_cct, 5) << "r=" << r << dendl;
 
   if (r < 0) {
+    if (r == -ECHILD) {
+      r = -ENOTEMPTY;
+    }
     send_close_image(r);
     return;
   }
diff --git a/src/librbd/operation/SnapshotRemoveRequest.cc b/src/librbd/operation/SnapshotRemoveRequest.cc
index 8c34ebf6c5342..072867b448640 100644
--- a/src/librbd/operation/SnapshotRemoveRequest.cc
+++ b/src/librbd/operation/SnapshotRemoveRequest.cc
@@ -55,7 +55,7 @@ bool SnapshotRemoveRequest<I>::should_complete(int r) {
   I &image_ctx = this->m_image_ctx;
   CephContext *cct = image_ctx.cct;
   ldout(cct, 5) << "r=" << r << dendl;
-  if (r < 0) {
+  if (r < 0 && r != -EBUSY) {
     lderr(cct) << "encountered error: " << cpp_strerror(r) << dendl;
   }
   return true;
diff --git a/src/test/librbd/image/test_mock_DetachChildRequest.cc b/src/test/librbd/image/test_mock_DetachChildRequest.cc
index 37f38c63c9a81..9c8618b779579 100644
--- a/src/test/librbd/image/test_mock_DetachChildRequest.cc
+++ b/src/test/librbd/image/test_mock_DetachChildRequest.cc
@@ -7,7 +7,9 @@
 #include "test/librbd/mock/MockContextWQ.h"
 #include "test/librados_test_stub/MockTestMemIoCtxImpl.h"
 #include "test/librados_test_stub/MockTestMemRadosClient.h"
+#include "librbd/image/TypeTraits.h"
 #include "librbd/image/DetachChildRequest.h"
+#include "librbd/trash/RemoveRequest.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -32,6 +34,46 @@ struct MockTestImageCtx : public MockImageCtx {
 MockTestImageCtx* MockTestImageCtx::s_instance = nullptr;
 
 } // anonymous namespace
+
+namespace image {
+
+template <>
+struct TypeTraits<MockTestImageCtx> {
+  typedef librbd::MockContextWQ ContextWQ;
+};
+
+} // namespace image
+
+namespace trash {
+
+template <>
+class RemoveRequest<MockTestImageCtx> {
+private:
+  typedef ::librbd::image::TypeTraits<MockTestImageCtx> TypeTraits;
+  typedef typename TypeTraits::ContextWQ ContextWQ;
+public:
+  static RemoveRequest *s_instance;
+  static RemoveRequest *create(librados::IoCtx &ioctx,
+                               MockTestImageCtx *image_ctx,
+                               ContextWQ *op_work_queue, bool force,
+                               ProgressContext &prog_ctx, Context *on_finish) {
+    ceph_assert(s_instance != nullptr);
+    s_instance->on_finish = on_finish;
+    return s_instance;
+  }
+
+  Context *on_finish = nullptr;
+
+  RemoveRequest() {
+    s_instance = this;
+  }
+
+  MOCK_METHOD0(send, void());
+};
+
+RemoveRequest<MockTestImageCtx> *RemoveRequest<MockTestImageCtx>::s_instance;
+
+} // namespace trash
 } // namespace librbd
 
 // template definitions
@@ -52,6 +94,7 @@ using ::testing::WithArg;
 class TestMockImageDetachChildRequest : public TestMockFixture {
 public:
   typedef DetachChildRequest<MockTestImageCtx> MockDetachChildRequest;
+  typedef trash::RemoveRequest<MockTestImageCtx> MockTrashRemoveRequest;
 
   void SetUp() override {
     TestMockFixture::SetUp();
@@ -138,6 +181,28 @@ public:
                            })));
   }
 
+  void expect_trash_get(MockImageCtx &mock_image_ctx,
+                        librados::MockTestMemIoCtxImpl &mock_io_ctx_impl,
+                        const cls::rbd::TrashImageSpec& trash_spec,
+                        int r) {
+    using ceph::encode;
+    EXPECT_CALL(mock_io_ctx_impl,
+                exec(RBD_TRASH, _, StrEq("rbd"),
+                     StrEq("trash_get"), _, _, _))
+      .WillOnce(WithArg<5>(Invoke([trash_spec, r](bufferlist* bl) {
+                             encode(trash_spec, *bl);
+                             return r;
+                           })));
+  }
+
+  void expect_trash_remove(MockTrashRemoveRequest& mock_trash_remove_request,
+                           int r) {
+    EXPECT_CALL(mock_trash_remove_request, send())
+      .WillOnce(Invoke([&mock_trash_remove_request, r]() {
+          mock_trash_remove_request.on_finish->complete(r);
+        }));
+  }
+
   librbd::ImageCtx *image_ctx;
 };
 
@@ -198,6 +263,8 @@ TEST_F(TestMockImageDetachChildRequest, TrashedSnapshotSuccess) {
                        "snap1", 123, {}, 0}, 0);
   expect_open(mock_image_ctx, 0);
   expect_snap_remove(mock_image_ctx, "snap1", 0);
+  const cls::rbd::TrashImageSpec trash_spec;
+  expect_trash_get(mock_image_ctx, *mock_io_ctx_impl, trash_spec, -ENOENT);
   expect_close(mock_image_ctx, 0);
 
   C_SaferCond ctx;
@@ -206,6 +273,37 @@ TEST_F(TestMockImageDetachChildRequest, TrashedSnapshotSuccess) {
   ASSERT_EQ(0, ctx.wait());
 }
 
+TEST_F(TestMockImageDetachChildRequest, ParentAutoRemove) {
+  REQUIRE_FEATURE(RBD_FEATURE_LAYERING);
+
+  MockTestImageCtx mock_image_ctx(*image_ctx);
+  mock_image_ctx.parent_md.spec = {m_ioctx.get_id(), "", "parent id", 234};
+
+  InSequence seq;
+  expect_test_op_features(mock_image_ctx, true);
+
+  librados::MockTestMemIoCtxImpl *mock_io_ctx_impl;
+  expect_create_ioctx(mock_image_ctx, &mock_io_ctx_impl);
+  expect_child_detach(mock_image_ctx, *mock_io_ctx_impl, 0);
+  expect_snapshot_get(mock_image_ctx, *mock_io_ctx_impl,
+                      "rbd_header.parent id",
+                      {234, {cls::rbd::TrashSnapshotNamespace{}},
+                       "snap1", 123, {}, 0}, 0);
+  expect_open(mock_image_ctx, 0);
+  expect_snap_remove(mock_image_ctx, "snap1", 0);
+  const cls::rbd::TrashImageSpec trash_spec =
+      {cls::rbd::TRASH_IMAGE_SOURCE_USER_PARENT, "parent", {}, {}};
+
+  expect_trash_get(mock_image_ctx, *mock_io_ctx_impl, trash_spec, 0);
+  MockTrashRemoveRequest mock_trash_remove_request;
+  expect_trash_remove(mock_trash_remove_request, 0);
+
+  C_SaferCond ctx;
+  auto req = MockDetachChildRequest::create(mock_image_ctx, &ctx);
+  req->send();
+  ASSERT_EQ(0, ctx.wait());
+}
+
 TEST_F(TestMockImageDetachChildRequest, TrashedSnapshotInUse) {
   REQUIRE_FEATURE(RBD_FEATURE_LAYERING);
 
diff --git a/src/tools/rbd/action/Trash.cc b/src/tools/rbd/action/Trash.cc
index 327b20ba703b9..d9900611c2976 100644
--- a/src/tools/rbd/action/Trash.cc
+++ b/src/tools/rbd/action/Trash.cc
@@ -38,7 +38,8 @@ static const std::string EXPIRED_BEFORE("expired-before");
 static const std::string THRESHOLD("threshold");
 
 static bool is_not_trash_user(const librbd::trash_image_info_t &trash_info) {
-  return trash_info.source != RBD_TRASH_IMAGE_SOURCE_USER;
+  return trash_info.source != RBD_TRASH_IMAGE_SOURCE_USER &&
+    trash_info.source != RBD_TRASH_IMAGE_SOURCE_USER_PARENT;
 }
 
 void get_move_arguments(po::options_description *positional,
@@ -269,6 +270,9 @@ int do_list(librbd::RBD &rbd, librados::IoCtx& io_ctx, bool long_flag,
       case RBD_TRASH_IMAGE_SOURCE_REMOVING:
         del_source = "REMOVING";
         break;
+      case RBD_TRASH_IMAGE_SOURCE_USER_PARENT:
+        del_source = "USER_PARENT";
+        break;
     }
 
     std::string time_str = ctime(&entry.deletion_time);
-- 
2.47.3