From cc0ed06e48dc4268a3c94e78e2567f976097bbf1 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Wed, 8 Jul 2015 13:46:00 -0400 Subject: [PATCH] tests: update librbd tests to support new AIO / lock handling ImageWatcher is no longer responsible for queueing write ops while waiting for the exclusive lock. Signed-off-by: Jason Dillaman --- src/test/librbd/test_ImageWatcher.cc | 232 ++++++++++++--------------- src/test/librbd/test_internal.cc | 13 +- 2 files changed, 105 insertions(+), 140 deletions(-) diff --git a/src/test/librbd/test_ImageWatcher.cc b/src/test/librbd/test_ImageWatcher.cc index d0a76e36a98a0..4c72913e2d8ec 100644 --- a/src/test/librbd/test_ImageWatcher.cc +++ b/src/test/librbd/test_ImageWatcher.cc @@ -13,6 +13,7 @@ #include "cls/lock/cls_lock_client.h" #include "cls/lock/cls_lock_types.h" #include "librbd/AioCompletion.h" +#include "librbd/AioImageRequestWQ.h" #include "librbd/internal.h" #include "librbd/ImageCtx.h" #include "librbd/ImageWatcher.h" @@ -40,9 +41,7 @@ void register_test_image_watcher() { class TestImageWatcher : public TestFixture { public: - TestImageWatcher() : m_watch_ctx(NULL), m_aio_completion_restarts(0), - m_expected_aio_restarts(0), - m_callback_lock("m_callback_lock") + TestImageWatcher() : m_watch_ctx(NULL), m_callback_lock("m_callback_lock") { } @@ -73,8 +72,10 @@ public: DECODE_FINISH(iter); NotifyOp notify_op = static_cast(op); + /* std::cout << "NOTIFY: " << notify_op << ", " << notify_id << ", " << cookie << ", " << notifier_id << std::endl; + */ Mutex::Locker l(m_parent.m_callback_lock); m_parent.m_notify_payloads[notify_op] = payload; @@ -143,65 +144,6 @@ public: return (m_notifies.size() == m_notify_acks.size()); } - - librbd::AioCompletion *create_aio_completion(librbd::ImageCtx &ictx) { - librbd::AioCompletion *aio_completion = new librbd::AioCompletion(); - aio_completion->complete_cb = &handle_aio_completion; - aio_completion->complete_arg = this; - - aio_completion->init_time(&ictx, librbd::AIO_TYPE_NONE); - m_aio_completions.insert(aio_completion); - return aio_completion; - } - - static void handle_aio_completion(void *arg1, void *arg2) { - TestImageWatcher *test_image_watcher = - reinterpret_cast(arg2); - assert(test_image_watcher->m_callback_lock.is_locked()); - test_image_watcher->m_callback_cond.Signal(); - } - - int handle_restart_aio(librbd::ImageCtx *ictx, - librbd::AioCompletion *aio_completion) { - assert(ictx->owner_lock.is_locked()); - Mutex::Locker callback_locker(m_callback_lock); - ++m_aio_completion_restarts; - - if (!ictx->image_watcher->is_lock_owner() && - (m_expected_aio_restarts == 0 || - m_aio_completion_restarts < m_expected_aio_restarts)) { - ictx->image_watcher->request_lock( - boost::bind(&TestImageWatcher::handle_restart_aio, this, ictx, _1), - aio_completion); - } else { - { - Mutex::Locker completion_locker(aio_completion->lock); - aio_completion->complete(ictx->cct); - } - - m_aio_completions.erase(aio_completion); - aio_completion->release(); - } - - m_callback_cond.Signal(); - return 0; - } - - bool wait_for_aio_completions(librbd::ImageCtx &ictx) { - Mutex::Locker l(m_callback_lock); - int r = 0; - while (!m_aio_completions.empty() && - (m_expected_aio_restarts == 0 || - m_aio_completion_restarts < m_expected_aio_restarts)) { - r = m_callback_cond.WaitInterval(ictx.cct, m_callback_lock, - utime_t(10, 0)); - if (r != 0) { - break; - } - } - return (r == 0); - } - bufferlist create_response_message(int r) { bufferlist bl; ::encode(ResponseMessage(r), bl); @@ -269,10 +211,6 @@ public: AsyncRequestId m_async_request_id; - std::set m_aio_completions; - uint32_t m_aio_completion_restarts; - uint32_t m_expected_aio_restarts; - Mutex m_callback_lock; Cond m_callback_cond; @@ -373,6 +311,14 @@ TEST_F(TestImageWatcher, IsLockSupported) { ictx->snap_id = CEPH_NOSNAP; } +TEST_F(TestImageWatcher, WritesSuspended) { + REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_TRUE(ictx->aio_work_queue->writes_suspended()); +} + TEST_F(TestImageWatcher, TryLock) { REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK); @@ -548,14 +494,10 @@ TEST_F(TestImageWatcher, RequestLock) { m_notify_acks = {{NOTIFY_OP_REQUEST_LOCK, create_response_message(0)}}; + ASSERT_TRUE(ictx->aio_work_queue->writes_suspended()); { - RWLock::WLocker l(ictx->owner_lock); - ictx->image_watcher->request_lock( - boost::bind(&TestImageWatcher::handle_restart_aio, this, ictx, _1), - create_aio_completion(*ictx)); - ictx->image_watcher->request_lock( - boost::bind(&TestImageWatcher::handle_restart_aio, this, ictx, _1), - create_aio_completion(*ictx)); + RWLock::RLocker owner_locker(ictx->owner_lock); + ictx->image_watcher->request_lock(); } ASSERT_TRUE(wait_for_notifies(*ictx)); @@ -565,8 +507,12 @@ TEST_F(TestImageWatcher, RequestLock) { ASSERT_EQ(0, unlock_image()); - m_notifies.clear(); - m_notify_acks = {{NOTIFY_OP_RELEASED_LOCK,{}}, {NOTIFY_OP_ACQUIRED_LOCK,{}}}; + { + Mutex::Locker l(m_callback_lock); + m_notifies.clear(); + m_notify_acks = {{NOTIFY_OP_RELEASED_LOCK,{}}, + {NOTIFY_OP_ACQUIRED_LOCK,{}}}; + } bufferlist bl; { @@ -581,7 +527,7 @@ TEST_F(TestImageWatcher, RequestLock) { expected_notify_ops += NOTIFY_OP_RELEASED_LOCK, NOTIFY_OP_ACQUIRED_LOCK; ASSERT_EQ(expected_notify_ops, m_notifies); - ASSERT_TRUE(wait_for_aio_completions(*ictx)); + ASSERT_FALSE(ictx->aio_work_queue->writes_suspended()); } TEST_F(TestImageWatcher, RequestLockTimedOut) { @@ -595,12 +541,10 @@ TEST_F(TestImageWatcher, RequestLockTimedOut) { m_notify_acks = {{NOTIFY_OP_REQUEST_LOCK, {}}}; - m_expected_aio_restarts = 1; + ASSERT_TRUE(ictx->aio_work_queue->writes_suspended()); { - RWLock::WLocker l(ictx->owner_lock); - ictx->image_watcher->request_lock( - boost::bind(&TestImageWatcher::handle_restart_aio, this, ictx, _1), - create_aio_completion(*ictx)); + RWLock::RLocker owner_locker(ictx->owner_lock); + ictx->image_watcher->request_lock(); } ASSERT_TRUE(wait_for_notifies(*ictx)); @@ -608,7 +552,23 @@ TEST_F(TestImageWatcher, RequestLockTimedOut) { expected_notify_ops += NOTIFY_OP_REQUEST_LOCK; ASSERT_EQ(expected_notify_ops, m_notifies); - ASSERT_TRUE(wait_for_aio_completions(*ictx)); + // should resend when empty ack returned + { + Mutex::Locker l(m_callback_lock); + m_notifies.clear(); + } + ASSERT_TRUE(wait_for_notifies(*ictx)); + ASSERT_TRUE(ictx->aio_work_queue->writes_suspended()); + + { + Mutex::Locker l(m_callback_lock); + ASSERT_EQ(0, unlock_image()); + m_notifies.clear(); + m_notify_acks = {{NOTIFY_OP_ACQUIRED_LOCK, bufferlist()}}; + } + + ASSERT_TRUE(wait_for_notifies(*ictx)); + ASSERT_FALSE(ictx->aio_work_queue->writes_suspended()); } TEST_F(TestImageWatcher, RequestLockIgnored) { @@ -629,11 +589,10 @@ TEST_F(TestImageWatcher, RequestLockIgnored) { stringify(orig_notify_timeout)); } BOOST_SCOPE_EXIT_END; + ASSERT_TRUE(ictx->aio_work_queue->writes_suspended()); { - RWLock::WLocker l(ictx->owner_lock); - ictx->image_watcher->request_lock( - boost::bind(&TestImageWatcher::handle_restart_aio, this, ictx, _1), - create_aio_completion(*ictx)); + RWLock::RLocker owner_locker(ictx->owner_lock); + ictx->image_watcher->request_lock(); } ASSERT_TRUE(wait_for_notifies(*ictx)); @@ -642,11 +601,22 @@ TEST_F(TestImageWatcher, RequestLockIgnored) { ASSERT_EQ(expected_notify_ops, m_notifies); // after the request times out -- it will be resent + { + Mutex::Locker l(m_callback_lock); + m_notifies.clear(); + } ASSERT_TRUE(wait_for_notifies(*ictx)); ASSERT_EQ(expected_notify_ops, m_notifies); - ASSERT_EQ(0, unlock_image()); - ASSERT_TRUE(wait_for_aio_completions(*ictx)); + { + Mutex::Locker l(m_callback_lock); + ASSERT_EQ(0, unlock_image()); + m_notifies.clear(); + m_notify_acks = {{NOTIFY_OP_ACQUIRED_LOCK, bufferlist()}}; + } + + ASSERT_TRUE(wait_for_notifies(*ictx)); + ASSERT_FALSE(ictx->aio_work_queue->writes_suspended()); } TEST_F(TestImageWatcher, RequestLockTryLockRace) { @@ -660,12 +630,10 @@ TEST_F(TestImageWatcher, RequestLockTryLockRace) { m_notify_acks = {{NOTIFY_OP_REQUEST_LOCK, create_response_message(0)}}; - m_expected_aio_restarts = 1; { - RWLock::WLocker l(ictx->owner_lock); - ictx->image_watcher->request_lock( - boost::bind(&TestImageWatcher::handle_restart_aio, this, ictx, _1), - create_aio_completion(*ictx)); + RWLock::RLocker owner_locker(ictx->owner_lock); + ictx->image_watcher->flag_aio_ops_pending(); + ictx->image_watcher->request_lock(); } ASSERT_TRUE(wait_for_notifies(*ictx)); @@ -673,8 +641,11 @@ TEST_F(TestImageWatcher, RequestLockTryLockRace) { expected_notify_ops += NOTIFY_OP_REQUEST_LOCK; ASSERT_EQ(expected_notify_ops, m_notifies); - m_notifies.clear(); - m_notify_acks = {{NOTIFY_OP_RELEASED_LOCK, {}}}; + { + Mutex::Locker l(m_callback_lock); + m_notifies.clear(); + m_notify_acks = {{NOTIFY_OP_RELEASED_LOCK, {}}}; + } bufferlist bl; { @@ -683,45 +654,43 @@ TEST_F(TestImageWatcher, RequestLockTryLockRace) { ENCODE_FINISH(bl); } ASSERT_EQ(0, m_ioctx.notify2(ictx->header_oid, bl, 5000, NULL)); - ASSERT_TRUE(wait_for_aio_completions(*ictx)); - RWLock::RLocker l(ictx->owner_lock); - ASSERT_FALSE(ictx->image_watcher->is_lock_owner()); -} -TEST_F(TestImageWatcher, RequestLockPreTryLockFailed) { - REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK); + // after losing race -- it will re-request + ASSERT_TRUE(wait_for_notifies(*ictx)); - librbd::ImageCtx *ictx; - ASSERT_EQ(0, open_image(m_image_name, &ictx)); - ASSERT_EQ(0, lock_image(*ictx, LOCK_SHARED, "manually 1234")); + { + RWLock::RLocker owner_locker(ictx->owner_lock); + ASSERT_FALSE(ictx->image_watcher->is_lock_owner()); + } - m_expected_aio_restarts = 1; { - RWLock::WLocker l(ictx->owner_lock); - ictx->image_watcher->request_lock( - boost::bind(&TestImageWatcher::handle_restart_aio, this, ictx, _1), - create_aio_completion(*ictx)); + Mutex::Locker l(m_callback_lock); + ASSERT_EQ(0, unlock_image()); + m_notifies.clear(); + m_notify_acks = { + {NOTIFY_OP_RELEASED_LOCK, bufferlist()}, + {NOTIFY_OP_ACQUIRED_LOCK, bufferlist()}}; } - ASSERT_TRUE(wait_for_aio_completions(*ictx)); + + ASSERT_EQ(0, m_ioctx.notify2(ictx->header_oid, bl, 5000, NULL)); + ASSERT_TRUE(wait_for_notifies(*ictx)); + ASSERT_FALSE(ictx->aio_work_queue->writes_suspended()); } -TEST_F(TestImageWatcher, RequestLockPostTryLockFailed) { +TEST_F(TestImageWatcher, RequestLockTryLockFailed) { REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK); librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); ASSERT_EQ(0, register_image_watch(*ictx)); - ASSERT_EQ(0, lock_image(*ictx, LOCK_EXCLUSIVE, - "auto " + stringify(m_watch_ctx->get_handle()))); + ASSERT_EQ(0, lock_image(*ictx, LOCK_SHARED, "manually 1234")); - m_notify_acks = {{NOTIFY_OP_REQUEST_LOCK, create_response_message(0)}}; + m_notify_acks = {{NOTIFY_OP_REQUEST_LOCK, {}}}; - m_expected_aio_restarts = 1; + ASSERT_TRUE(ictx->aio_work_queue->writes_suspended()); { - RWLock::WLocker l(ictx->owner_lock); - ictx->image_watcher->request_lock( - boost::bind(&TestImageWatcher::handle_restart_aio, this, ictx, _1), - create_aio_completion(*ictx)); + RWLock::RLocker owner_locker(ictx->owner_lock); + ictx->image_watcher->request_lock(); } ASSERT_TRUE(wait_for_notifies(*ictx)); @@ -729,20 +698,23 @@ TEST_F(TestImageWatcher, RequestLockPostTryLockFailed) { expected_notify_ops += NOTIFY_OP_REQUEST_LOCK; ASSERT_EQ(expected_notify_ops, m_notifies); - ASSERT_EQ(0, unlock_image()); - ASSERT_EQ(0, lock_image(*ictx, LOCK_SHARED, "manually 1234")); - - m_notifies.clear(); - m_notify_acks = {{NOTIFY_OP_RELEASED_LOCK, bufferlist()}}; + // should resend when error encountered + { + Mutex::Locker l(m_callback_lock); + m_notifies.clear(); + } + ASSERT_TRUE(wait_for_notifies(*ictx)); + ASSERT_TRUE(ictx->aio_work_queue->writes_suspended()); - bufferlist bl; { - ENCODE_START(1, 1, bl); - ::encode(NOTIFY_OP_RELEASED_LOCK, bl); - ENCODE_FINISH(bl); + Mutex::Locker l(m_callback_lock); + ASSERT_EQ(0, unlock_image()); + m_notifies.clear(); + m_notify_acks = {{NOTIFY_OP_ACQUIRED_LOCK, bufferlist()}}; } - ASSERT_EQ(0, m_ioctx.notify2(ictx->header_oid, bl, 5000, NULL)); - ASSERT_TRUE(wait_for_aio_completions(*ictx)); + + ASSERT_TRUE(wait_for_notifies(*ictx)); + ASSERT_FALSE(ictx->aio_work_queue->writes_suspended()); } TEST_F(TestImageWatcher, NotifyHeaderUpdate) { diff --git a/src/test/librbd/test_internal.cc b/src/test/librbd/test_internal.cc index 5269005c41832..559bf2b398498 100644 --- a/src/test/librbd/test_internal.cc +++ b/src/test/librbd/test_internal.cc @@ -256,11 +256,7 @@ TEST_F(TestInternal, AioWriteRequestsLock) { librbd::AioCompletion *c = librbd::aio_create_completion_internal(ctx, librbd::rbd_ctx_cb); c->get(); - { - RWLock::RLocker owner_lock(ictx->owner_lock); - librbd::AioImageRequest::write(ictx, c, 0, buffer.size(), buffer.c_str(), - 0); - } + ictx->aio_work_queue->aio_write(c, 0, buffer.size(), buffer.c_str(), 0); bool is_owner; ASSERT_EQ(0, librbd::is_exclusive_lock_owner(ictx, &is_owner)); @@ -283,10 +279,7 @@ TEST_F(TestInternal, AioDiscardRequestsLock) { librbd::AioCompletion *c = librbd::aio_create_completion_internal(ctx, librbd::rbd_ctx_cb); c->get(); - { - RWLock::RLocker owner_lock(ictx->owner_lock); - librbd::AioImageRequest::discard(ictx, c, 0, 256); - } + ictx->aio_work_queue->aio_discard(c, 0, 256); bool is_owner; ASSERT_EQ(0, librbd::is_exclusive_lock_owner(ictx, &is_owner)); @@ -673,7 +666,7 @@ TEST_F(TestInternal, ShrinkFlushesCache) { librbd::AioCompletion *c = librbd::aio_create_completion_internal(&cond_ctx, librbd::rbd_ctx_cb); c->get(); - aio_write(ictx, 0, buffer.size(), buffer.c_str(), c, 0); + ictx->aio_work_queue->aio_write(c, 0, buffer.size(), buffer.c_str(), 0); librbd::NoOpProgressContext no_op; ASSERT_EQ(0, librbd::resize(ictx, m_image_size >> 1, no_op)); -- 2.39.5