From 96e6639b135be4306ee123da2288d537862c796a Mon Sep 17 00:00:00 2001 From: Mykola Golub Date: Sat, 9 Jun 2018 13:28:52 +0300 Subject: [PATCH] librbd: don't update object map if object deep copy returned ENOENT Signed-off-by: Mykola Golub --- src/librbd/deep_copy/ImageCopyRequest.cc | 2 +- src/librbd/deep_copy/ObjectCopyRequest.cc | 2 +- src/librbd/io/CopyupRequest.cc | 33 +++++++++++-------- src/librbd/io/CopyupRequest.h | 4 +-- src/librbd/operation/MigrateRequest.cc | 4 +++ .../deep_copy/test_mock_ObjectCopyRequest.cc | 2 +- 6 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/librbd/deep_copy/ImageCopyRequest.cc b/src/librbd/deep_copy/ImageCopyRequest.cc index 651937fcd36ff..8762fc15c66fc 100644 --- a/src/librbd/deep_copy/ImageCopyRequest.cc +++ b/src/librbd/deep_copy/ImageCopyRequest.cc @@ -200,7 +200,7 @@ void ImageCopyRequest::handle_object_copy(uint64_t object_no, int r) { assert(m_current_ops > 0); --m_current_ops; - if (r < 0) { + if (r < 0 && r != -ENOENT) { lderr(m_cct) << "object copy failed: " << cpp_strerror(r) << dendl; if (m_ret_val == 0) { m_ret_val = r; diff --git a/src/librbd/deep_copy/ObjectCopyRequest.cc b/src/librbd/deep_copy/ObjectCopyRequest.cc index 3d6bbe8404e0a..235263ac107be 100644 --- a/src/librbd/deep_copy/ObjectCopyRequest.cc +++ b/src/librbd/deep_copy/ObjectCopyRequest.cc @@ -284,7 +284,7 @@ void ObjectCopyRequest::handle_read_from_parent(int r) { if (m_write_ops.empty()) { // nothing to copy - finish(0); + finish(-ENOENT); return; } diff --git a/src/librbd/io/CopyupRequest.cc b/src/librbd/io/CopyupRequest.cc index 3addb8f2c1ed4..77ffaace2719e 100644 --- a/src/librbd/io/CopyupRequest.cc +++ b/src/librbd/io/CopyupRequest.cc @@ -205,7 +205,11 @@ bool CopyupRequest::is_copyup_required() { } template -bool CopyupRequest::is_update_object_map_required() { +bool CopyupRequest::is_update_object_map_required(int r) { + if (r < 0) { + return false; + } + RWLock::RLocker owner_locker(m_ictx->owner_lock); RWLock::RLocker snap_locker(m_ictx->snap_lock); if (m_ictx->object_map == nullptr) { @@ -259,26 +263,28 @@ void CopyupRequest::send() template void CopyupRequest::complete(int r) { - if (should_complete(r)) { + if (should_complete(&r)) { complete_requests(r); delete this; } } template -bool CopyupRequest::should_complete(int r) -{ +bool CopyupRequest::should_complete(int *r) { CephContext *cct = m_ictx->cct; ldout(cct, 20) << "oid " << m_oid - << ", r " << r << dendl; + << ", r " << *r << dendl; uint64_t pending_copyups; switch (m_state) { case STATE_READ_FROM_PARENT: ldout(cct, 20) << "READ_FROM_PARENT" << dendl; remove_from_list(); - if (r >= 0 || r == -ENOENT) { - if (!is_copyup_required() && !is_update_object_map_required()) { + if (*r >= 0 || *r == -ENOENT) { + if (!is_copyup_required() && !is_update_object_map_required(*r)) { + if (*r == -ENOENT && is_deep_copy()) { + *r = 0; + } ldout(cct, 20) << "skipping" << dendl; return true; } @@ -289,12 +295,12 @@ bool CopyupRequest::should_complete(int r) case STATE_OBJECT_MAP_HEAD: ldout(cct, 20) << "OBJECT_MAP_HEAD" << dendl; - assert(r == 0); + assert(*r == 0); return send_object_map(); case STATE_OBJECT_MAP: ldout(cct, 20) << "OBJECT_MAP" << dendl; - assert(r == 0); + assert(*r == 0); if (!is_copyup_required()) { ldout(cct, 20) << "skipping copyup" << dendl; return true; @@ -309,13 +315,14 @@ bool CopyupRequest::should_complete(int r) } ldout(cct, 20) << "COPYUP (" << pending_copyups << " pending)" << dendl; - if (r == -ENOENT) { + if (*r == -ENOENT) { // hide the -ENOENT error if this is the last op if (pending_copyups == 0) { + *r = 0; complete_requests(0); } - } else if (r < 0) { - complete_requests(r); + } else if (*r < 0) { + complete_requests(*r); } return (pending_copyups == 0); @@ -324,7 +331,7 @@ bool CopyupRequest::should_complete(int r) ceph_abort(); break; } - return (r < 0); + return (*r < 0); } template diff --git a/src/librbd/io/CopyupRequest.h b/src/librbd/io/CopyupRequest.h index c92494c2f9989..31eae5c1ac6c6 100644 --- a/src/librbd/io/CopyupRequest.h +++ b/src/librbd/io/CopyupRequest.h @@ -105,7 +105,7 @@ private: void complete_requests(int r); - bool should_complete(int r); + bool should_complete(int *r); void remove_from_list(); @@ -113,7 +113,7 @@ private: bool send_object_map(); bool send_copyup(); bool is_copyup_required(); - bool is_update_object_map_required(); + bool is_update_object_map_required(int r); bool is_deep_copy() const; }; diff --git a/src/librbd/operation/MigrateRequest.cc b/src/librbd/operation/MigrateRequest.cc index d736a8668b89b..f7ec41621d662 100644 --- a/src/librbd/operation/MigrateRequest.cc +++ b/src/librbd/operation/MigrateRequest.cc @@ -141,6 +141,10 @@ private: CephContext *cct = this->m_image_ctx.cct; ldout(cct, 10) << "r=" << r << dendl; + if (r == -ENOENT) { + r = 0; + } + m_async_op.finish_op(); this->complete(r); } diff --git a/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc b/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc index 0a1afcd1819d5..a52c62fbb060e 100644 --- a/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc +++ b/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc @@ -464,7 +464,7 @@ TEST_F(TestMockDeepCopyObjectCopyRequest, DNE) { expect_list_snaps(mock_src_image_ctx, mock_src_io_ctx, -ENOENT); request->send(); - ASSERT_EQ(0, ctx.wait()); + ASSERT_EQ(-ENOENT, ctx.wait()); } TEST_F(TestMockDeepCopyObjectCopyRequest, Write) { -- 2.39.5