From 0e33f1c9ed3643adf274f1733882aa539bee5d98 Mon Sep 17 00:00:00 2001
From: Jason Dillaman <dillaman@redhat.com>
Date: Mon, 9 Mar 2020 22:47:49 -0400
Subject: [PATCH] rbd-mirror: remove exclusive-lock requirement from snapshot
 purge

When using snapshot-based mirroring, there shouldn't be any need to
force the use of the exclusive-lock feature.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
---
 .../test_mock_SnapshotPurgeRequest.cc         | 44 ++++++++++++++++++-
 .../image_deleter/SnapshotPurgeRequest.cc     | 14 ++++--
 .../image_deleter/SnapshotPurgeRequest.h      |  1 +
 3 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/src/test/rbd_mirror/image_deleter/test_mock_SnapshotPurgeRequest.cc b/src/test/rbd_mirror/image_deleter/test_mock_SnapshotPurgeRequest.cc
index 627082e285c..ef0d0afb1c0 100644
--- a/src/test/rbd_mirror/image_deleter/test_mock_SnapshotPurgeRequest.cc
+++ b/src/test/rbd_mirror/image_deleter/test_mock_SnapshotPurgeRequest.cc
@@ -159,7 +159,7 @@ public:
   librbd::ImageCtx *m_local_image_ctx;
 };
 
-TEST_F(TestMockImageDeleterSnapshotPurgeRequest, Success) {
+TEST_F(TestMockImageDeleterSnapshotPurgeRequest, SuccessJournal) {
   {
     std::unique_lock image_locker{m_local_image_ctx->image_lock};
     m_local_image_ctx->add_snap(cls::rbd::UserSnapshotNamespace{}, "snap1", 1,
@@ -207,6 +207,48 @@ TEST_F(TestMockImageDeleterSnapshotPurgeRequest, Success) {
   ASSERT_EQ(0, ctx.wait());
 }
 
+TEST_F(TestMockImageDeleterSnapshotPurgeRequest, SuccessSnapshot) {
+  {
+    std::unique_lock image_locker{m_local_image_ctx->image_lock};
+    m_local_image_ctx->add_snap(cls::rbd::UserSnapshotNamespace{}, "snap1", 1,
+                                0, {}, RBD_PROTECTION_STATUS_PROTECTED, 0, {});
+    m_local_image_ctx->add_snap(cls::rbd::UserSnapshotNamespace{}, "snap2", 2,
+                                0, {}, RBD_PROTECTION_STATUS_UNPROTECTED, 0,
+                                {});
+  }
+
+  librbd::MockTestImageCtx mock_image_ctx(*m_local_image_ctx);
+
+  InSequence seq;
+  expect_set_journal_policy(mock_image_ctx);
+  expect_open(mock_image_ctx, 0);
+
+  expect_get_snap_namespace(mock_image_ctx, 2,
+                            cls::rbd::UserSnapshotNamespace{}, 0);
+  expect_get_snap_name(mock_image_ctx, 2, "snap2", 0);
+  expect_is_snap_protected(mock_image_ctx, 2, false, 0);
+  expect_snap_remove(mock_image_ctx, cls::rbd::UserSnapshotNamespace{}, "snap2",
+                     0);
+
+  expect_get_snap_namespace(mock_image_ctx, 1,
+                            cls::rbd::UserSnapshotNamespace{}, 0);
+  expect_get_snap_name(mock_image_ctx, 1, "snap1", 0);
+  expect_is_snap_protected(mock_image_ctx, 1, true, 0);
+  expect_snap_unprotect(mock_image_ctx, cls::rbd::UserSnapshotNamespace{},
+                        "snap1", 0);
+  expect_snap_remove(mock_image_ctx, cls::rbd::UserSnapshotNamespace{}, "snap1",
+                     0);
+
+  expect_close(mock_image_ctx, 0);
+  expect_destroy(mock_image_ctx);
+
+  C_SaferCond ctx;
+  auto req = MockSnapshotPurgeRequest::create(m_local_io_ctx, mock_image_ctx.id,
+                                              &ctx);
+  req->send();
+  ASSERT_EQ(0, ctx.wait());
+}
+
 TEST_F(TestMockImageDeleterSnapshotPurgeRequest, OpenError) {
   {
     std::unique_lock image_locker{m_local_image_ctx->image_lock};
diff --git a/src/tools/rbd_mirror/image_deleter/SnapshotPurgeRequest.cc b/src/tools/rbd_mirror/image_deleter/SnapshotPurgeRequest.cc
index 8280e38ba18..642149c31ae 100644
--- a/src/tools/rbd_mirror/image_deleter/SnapshotPurgeRequest.cc
+++ b/src/tools/rbd_mirror/image_deleter/SnapshotPurgeRequest.cc
@@ -73,9 +73,7 @@ void SnapshotPurgeRequest<I>::acquire_lock() {
   if (m_image_ctx->exclusive_lock == nullptr) {
     m_image_ctx->owner_lock.unlock_shared();
 
-    derr << "exclusive lock not enabled" << dendl;
-    m_ret_val = -EINVAL;
-    close_image();
+    start_snap_unprotect();
     return;
   }
 
@@ -96,6 +94,13 @@ void SnapshotPurgeRequest<I>::handle_acquire_lock(int r) {
     return;
   }
 
+  start_snap_unprotect();
+}
+
+template <typename I>
+void SnapshotPurgeRequest<I>::start_snap_unprotect() {
+  dout(10) << dendl;
+
   {
     std::shared_lock image_locker{m_image_ctx->image_lock};
     m_snaps = m_image_ctx->snaps;
@@ -283,6 +288,9 @@ void SnapshotPurgeRequest<I>::finish(int r) {
 template <typename I>
 Context *SnapshotPurgeRequest<I>::start_lock_op(int* r) {
   std::shared_lock owner_locker{m_image_ctx->owner_lock};
+  if (m_image_ctx->exclusive_lock == nullptr) {
+    return new LambdaContext([](int r) {});
+  }
   return m_image_ctx->exclusive_lock->start_op(r);
 }
 
diff --git a/src/tools/rbd_mirror/image_deleter/SnapshotPurgeRequest.h b/src/tools/rbd_mirror/image_deleter/SnapshotPurgeRequest.h
index b8b635fe765..70cae851820 100644
--- a/src/tools/rbd_mirror/image_deleter/SnapshotPurgeRequest.h
+++ b/src/tools/rbd_mirror/image_deleter/SnapshotPurgeRequest.h
@@ -79,6 +79,7 @@ private:
   void acquire_lock();
   void handle_acquire_lock(int r);
 
+  void start_snap_unprotect();
   void snap_unprotect();
   void handle_snap_unprotect(int r);
 
-- 
2.39.5