From: Or Ozeri Date: Wed, 20 Jan 2021 16:43:47 +0000 (+0200) Subject: librbd: don't restart empty copyups in crypto layer X-Git-Tag: v17.0.0~11^2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=cd6f3ccd3363f40c4fbe1c52bba9cc77e4162950;p=ceph.git librbd: don't restart empty copyups in crypto layer This commit fixes a bug where an empty parent copyup is restarted indefinitely. Signed-off-by: Or Ozeri --- diff --git a/src/librbd/crypto/CryptoObjectDispatch.cc b/src/librbd/crypto/CryptoObjectDispatch.cc index 9cac7140761da..244f52dec2bd2 100644 --- a/src/librbd/crypto/CryptoObjectDispatch.cc +++ b/src/librbd/crypto/CryptoObjectDispatch.cc @@ -201,6 +201,7 @@ struct C_UnalignedObjectWriteRequest : public Context { int* object_dispatch_flags; uint64_t* journal_tid; Context* on_finish; + bool may_copyup; ceph::bufferlist aligned_data; io::ReadExtents extents; uint64_t version; @@ -214,14 +215,15 @@ struct C_UnalignedObjectWriteRequest : public Context { IOContext io_context, int op_flags, int write_flags, std::optional assert_version, const ZTracer::Trace &parent_trace, int* object_dispatch_flags, - uint64_t* journal_tid,Context* on_dispatched + uint64_t* journal_tid, Context* on_dispatched, bool may_copyup ) : image_ctx(image_ctx), crypto(crypto), object_no(object_no), object_off(object_off), data(data), cmp_data(cmp_data), mismatch_offset(mismatch_offset), io_context(io_context), op_flags(op_flags), write_flags(write_flags), assert_version(assert_version), parent_trace(parent_trace), object_dispatch_flags(object_dispatch_flags), - journal_tid(journal_tid), on_finish(on_dispatched) { + journal_tid(journal_tid), on_finish(on_dispatched), + may_copyup(may_copyup) { // build read extents auto [pre_align, post_align] = crypto->get_pre_and_post_align( object_off, data.length()); @@ -337,7 +339,7 @@ struct C_UnalignedObjectWriteRequest : public Context { if (r < 0) { complete(r); } else { - restart_request(); + restart_request(false); } } @@ -345,13 +347,16 @@ struct C_UnalignedObjectWriteRequest : public Context { ldout(image_ctx->cct, 20) << "unaligned write r=" << r << dendl; if (r == -ENOENT) { - auto ctx = create_context_callback< - C_UnalignedObjectWriteRequest, - &C_UnalignedObjectWriteRequest::handle_copyup>(this); - if (io::util::trigger_copyup(image_ctx, object_no, io_context, ctx)) { - return; + if (may_copyup) { + auto ctx = create_context_callback< + C_UnalignedObjectWriteRequest, + &C_UnalignedObjectWriteRequest::handle_copyup>(this); + if (io::util::trigger_copyup( + image_ctx, object_no, io_context, ctx)) { + return; + } + delete ctx; } - delete ctx; object_exists = false; } else if (r < 0) { complete(r); @@ -388,13 +393,13 @@ struct C_UnalignedObjectWriteRequest : public Context { write_req->send(); } - void restart_request() { + void restart_request(bool may_copyup) { auto req = new C_UnalignedObjectWriteRequest( image_ctx, crypto, object_no, object_off, std::move(data), std::move(cmp_data), mismatch_offset, io_context, op_flags, write_flags, assert_version, parent_trace, - object_dispatch_flags, journal_tid, this); + object_dispatch_flags, journal_tid, this, may_copyup); req->send(); } @@ -409,7 +414,7 @@ struct C_UnalignedObjectWriteRequest : public Context { } if (restart) { - restart_request(); + restart_request(may_copyup); } else { complete(r); } @@ -491,7 +496,8 @@ bool CryptoObjectDispatch::write( auto req = new C_UnalignedObjectWriteRequest( m_image_ctx, m_crypto, object_no, object_off, std::move(data), {}, nullptr, io_context, op_flags, write_flags, assert_version, - parent_trace, object_dispatch_flags, journal_tid, on_dispatched); + parent_trace, object_dispatch_flags, journal_tid, on_dispatched, + true); req->send(); } @@ -552,7 +558,7 @@ bool CryptoObjectDispatch::compare_and_write( m_image_ctx, m_crypto, object_no, object_off, std::move(write_data), std::move(cmp_data), mismatch_offset, io_context, op_flags, 0, std::nullopt, parent_trace, object_dispatch_flags, journal_tid, - on_dispatched); + on_dispatched, true); req->send(); return true; diff --git a/src/test/librbd/crypto/test_mock_CryptoObjectDispatch.cc b/src/test/librbd/crypto/test_mock_CryptoObjectDispatch.cc index 2f6972217fdf0..f0160f1074a6e 100644 --- a/src/test/librbd/crypto/test_mock_CryptoObjectDispatch.cc +++ b/src/test/librbd/crypto/test_mock_CryptoObjectDispatch.cc @@ -543,6 +543,50 @@ TEST_F(TestMockCryptoCryptoObjectDispatch, UnalignedWriteCopyup) { ASSERT_EQ(0, dispatched_cond.wait()); } +TEST_F(TestMockCryptoCryptoObjectDispatch, UnalignedWriteEmptyCopyup) { + MockObjectMap mock_object_map; + mock_image_ctx->object_map = &mock_object_map; + MockExclusiveLock mock_exclusive_lock; + mock_image_ctx->exclusive_lock = &mock_exclusive_lock; + + ceph::bufferlist write_data; + write_data.append(std::string(8192, '1')); + io::ReadExtents extents = {{0, 4096}, {8192, 4096}}; + expect_object_read(&extents); + ASSERT_TRUE(mock_crypto_object_dispatch->write( + 0, 1, std::move(write_data), mock_image_ctx->get_data_io_context(), + 0, 0, std::nullopt, {}, nullptr, nullptr, &dispatch_result, + &on_finish, on_dispatched)); + ASSERT_EQ(dispatch_result, io::DISPATCH_RESULT_COMPLETE); + ASSERT_EQ(on_finish, &finished_cond); + + expect_get_object_size(); + expect_get_parent_overlap(mock_image_ctx->layout.object_size); + expect_remap_extents(0, mock_image_ctx->layout.object_size); + expect_prune_parent_extents(mock_image_ctx->layout.object_size); + EXPECT_CALL(mock_exclusive_lock, is_lock_owner()).WillRepeatedly( + Return(true)); + EXPECT_CALL(*mock_image_ctx->object_map, object_may_exist(0)).WillOnce( + Return(false)); + MockAbstractObjectWriteRequest *write_request = nullptr; + expect_copyup(&write_request, 0); + + // unaligned write restarted + expect_object_read(&extents); + dispatcher_ctx->complete(-ENOENT); // complete first read + ASSERT_EQ(ETIMEDOUT, dispatched_cond.wait_for(0)); + + auto expected_data = + std::string(1, '\0') + std::string(8192, '1') + + std::string(4095, '\0'); + expect_object_write(0, expected_data, io::OBJECT_WRITE_FLAG_CREATE_EXCLUSIVE, + std::nullopt); + dispatcher_ctx->complete(-ENOENT); // complete second read + ASSERT_EQ(ETIMEDOUT, dispatched_cond.wait_for(0)); + dispatcher_ctx->complete(0); // complete write + ASSERT_EQ(0, dispatched_cond.wait()); +} + TEST_F(TestMockCryptoCryptoObjectDispatch, UnalignedWriteFailVersionCheck) { ceph::bufferlist write_data; uint64_t version = 1234;