librbd: don't attempt to remove image state on orphan snapshots

Despite being mirror snapshots, orphan snapshots don't have image
state: see CreateNonPrimaryRequest::write_image_state() for a similar
is_orphan() check.  Attempting to remove image state generates bogus
"failed to read image state object" and "failed to remove image state"
errors.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
(cherry picked from commit cfae3f79bd0513e2753b0deb8c2624ab07cf2d1b)

Conflicts:
	src/librbd/operation/SnapshotRemoveRequest.cc [ commit
	  3a93b40721a1 ("librbd: s/boost::variant/std::variant/") not
	  in pacific ]

diff --git a/src/librbd/operation/SnapshotRemoveRequest.cc b/src/librbd/operation/SnapshotRemoveRequest.cc
index b78be8a0af6bb4fb403d52a9139ea31d263d5ca3..a8bb8fa7996214971146c8059d44cf125d8888d1 100644 (file)
--- a/src/librbd/operation/SnapshotRemoveRequest.cc
+++ b/src/librbd/operation/SnapshotRemoveRequest.cc
@@ -355,9 +355,10 @@ void SnapshotRemoveRequest<I>::handle_remove_object_map(int r) {
 template <typename I>
 void SnapshotRemoveRequest<I>::remove_image_state() {
   I &image_ctx = this->m_image_ctx;
-  auto type = cls::rbd::get_snap_namespace_type(m_snap_namespace);
 
-  if (type != cls::rbd::SNAPSHOT_NAMESPACE_TYPE_MIRROR) {
+  const auto* info = boost::get<cls::rbd::MirrorSnapshotNamespace>(
+    &m_snap_namespace);
+  if (info == nullptr || info->is_orphan()) {
     release_snap_id();
     return;
   }

diff --git a/src/test/librbd/operation/test_mock_SnapshotRemoveRequest.cc b/src/test/librbd/operation/test_mock_SnapshotRemoveRequest.cc
index 886ac768e74b2ad8223ff9c5cdd0e41effb2e260..d872a2112979ca6bea5a5a3b2b8bc29136a6784b 100644 (file)
--- a/src/test/librbd/operation/test_mock_SnapshotRemoveRequest.cc
+++ b/src/test/librbd/operation/test_mock_SnapshotRemoveRequest.cc
@@ -512,9 +512,10 @@ TEST_F(TestMockOperationSnapshotRemoveRequest, MirrorSnapshot) {
   expect_snapshot_trash_add(mock_image_ctx, 0);
 
   uint64_t snap_id = ictx->snap_info.rbegin()->first;
+  cls::rbd::MirrorSnapshotNamespace ns{
+    cls::rbd::MIRROR_SNAPSHOT_STATE_NON_PRIMARY, {}, "mirror uuid", 123};
   expect_snapshot_get(mock_image_ctx,
-                      {snap_id, {cls::rbd::MirrorSnapshotNamespace{}},
-                       "mirror", 123, {}, 0}, 0);
+                      {snap_id, {ns}, "mirror", 456, {}, 0}, 0);
 
   expect_get_parent_spec(mock_image_ctx, 0);
   expect_object_map_snap_remove(mock_image_ctx, 0);
@@ -526,8 +527,55 @@ TEST_F(TestMockOperationSnapshotRemoveRequest, MirrorSnapshot) {
 
   C_SaferCond cond_ctx;
   MockSnapshotRemoveRequest *req = new MockSnapshotRemoveRequest(
-    mock_image_ctx, &cond_ctx, cls::rbd::MirrorSnapshotNamespace(),
-    "mirror", snap_id);
+    mock_image_ctx, &cond_ctx, ns, "mirror", snap_id);
+  {
+    std::shared_lock owner_locker{mock_image_ctx.owner_lock};
+    req->send();
+  }
+  ASSERT_EQ(0, cond_ctx.wait());
+}
+
+TEST_F(TestMockOperationSnapshotRemoveRequest, MirrorSnapshotOrphan) {
+  REQUIRE_FORMAT_V2();
+
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+  ASSERT_EQ(0, snap_create(*ictx, "snap1"));
+  ASSERT_EQ(0, ictx->state->refresh_if_required());
+
+  MockImageCtx mock_image_ctx(*ictx);
+
+  MockExclusiveLock mock_exclusive_lock;
+  if (ictx->test_features(RBD_FEATURE_EXCLUSIVE_LOCK)) {
+    mock_image_ctx.exclusive_lock = &mock_exclusive_lock;
+  }
+
+  MockObjectMap mock_object_map;
+  if (ictx->test_features(RBD_FEATURE_OBJECT_MAP)) {
+    mock_image_ctx.object_map = &mock_object_map;
+  }
+
+  expect_op_work_queue(mock_image_ctx);
+
+  ::testing::InSequence seq;
+  expect_snapshot_trash_add(mock_image_ctx, 0);
+
+  uint64_t snap_id = ictx->snap_info.rbegin()->first;
+  cls::rbd::MirrorSnapshotNamespace ns{
+    cls::rbd::MIRROR_SNAPSHOT_STATE_NON_PRIMARY, {}, "", CEPH_NOSNAP};
+  expect_snapshot_get(mock_image_ctx,
+                      {snap_id, {ns}, "mirror", 456, {}, 0}, 0);
+
+  expect_get_parent_spec(mock_image_ctx, 0);
+  expect_object_map_snap_remove(mock_image_ctx, 0);
+  MockRemoveImageStateRequest mock_remove_image_state_request;
+  expect_release_snap_id(mock_image_ctx);
+  expect_snap_remove(mock_image_ctx, 0);
+  expect_rm_snap(mock_image_ctx);
+
+  C_SaferCond cond_ctx;
+  MockSnapshotRemoveRequest *req = new MockSnapshotRemoveRequest(
+    mock_image_ctx, &cond_ctx, ns, "mirror", snap_id);
   {
     std::shared_lock owner_locker{mock_image_ctx.owner_lock};
     req->send();