From 276fed6b7078158c2cd04fba14a11531c27898e0 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Mon, 6 Jun 2016 22:34:30 -0400 Subject: [PATCH] librbd: potential duplicate snap removal can result in crash Signed-off-by: Jason Dillaman --- src/librbd/operation/SnapshotRemoveRequest.cc | 9 +++++- .../test_mock_SnapshotRemoveRequest.cc | 31 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/librbd/operation/SnapshotRemoveRequest.cc b/src/librbd/operation/SnapshotRemoveRequest.cc index 1bdba71e3ced5..8ad123b2a1063 100644 --- a/src/librbd/operation/SnapshotRemoveRequest.cc +++ b/src/librbd/operation/SnapshotRemoveRequest.cc @@ -101,10 +101,17 @@ void SnapshotRemoveRequest::send_remove_object_map() { assert(image_ctx.owner_lock.is_locked()); { + CephContext *cct = image_ctx.cct; RWLock::WLocker snap_locker(image_ctx.snap_lock); RWLock::RLocker object_map_locker(image_ctx.object_map_lock); + if (image_ctx.snap_info.find(m_snap_id) == image_ctx.snap_info.end()) { + lderr(cct) << this << " " << __func__ << ": snapshot doesn't exist" + << dendl; + this->async_complete(-ENOENT); + return; + } + if (image_ctx.object_map != nullptr) { - CephContext *cct = image_ctx.cct; ldout(cct, 5) << this << " " << __func__ << dendl; m_state = STATE_REMOVE_OBJECT_MAP; diff --git a/src/test/librbd/operation/test_mock_SnapshotRemoveRequest.cc b/src/test/librbd/operation/test_mock_SnapshotRemoveRequest.cc index 4dcf3c8b44f35..67a0e6deccceb 100644 --- a/src/test/librbd/operation/test_mock_SnapshotRemoveRequest.cc +++ b/src/test/librbd/operation/test_mock_SnapshotRemoveRequest.cc @@ -356,5 +356,36 @@ TEST_F(TestMockOperationSnapshotRemoveRequest, RemoveSnapError) { ASSERT_EQ(-ENOENT, cond_ctx.wait()); } +TEST_F(TestMockOperationSnapshotRemoveRequest, MissingSnap) { + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + 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; + uint64_t snap_id = 456; + + C_SaferCond cond_ctx; + MockSnapshotRemoveRequest *req = new MockSnapshotRemoveRequest( + mock_image_ctx, &cond_ctx, "snap1", snap_id); + { + RWLock::RLocker owner_locker(mock_image_ctx.owner_lock); + req->send(); + } + ASSERT_EQ(-ENOENT, cond_ctx.wait()); +} + } // namespace operation } // namespace librbd