Merge pull request #26523 from dillaman/wip-38387

librbd: add missing shutdown states to managed lock helper

Reviewed-by: Mykola Golub <mgolub@suse.com>
This commit is contained in:
Mykola Golub 2019-02-27 20:49:09 +02:00 committed by GitHub
commit 17daad8536
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 179 additions and 61 deletions

View File

@ -323,6 +323,11 @@ cleanup()
if [ -z "${RBD_MIRROR_NOCLEANUP}" ]; then
local cluster instance
CEPH_ARGS='' ceph --cluster ${CLUSTER1} osd pool rm ${POOL} ${POOL} --yes-i-really-really-mean-it
CEPH_ARGS='' ceph --cluster ${CLUSTER2} osd pool rm ${POOL} ${POOL} --yes-i-really-really-mean-it
CEPH_ARGS='' ceph --cluster ${CLUSTER1} osd pool rm ${PARENT_POOL} ${PARENT_POOL} --yes-i-really-really-mean-it
CEPH_ARGS='' ceph --cluster ${CLUSTER2} osd pool rm ${PARENT_POOL} ${PARENT_POOL} --yes-i-really-really-mean-it
for cluster in "${CLUSTER1}" "${CLUSTER2}"; do
stop_mirrors "${cluster}"
done
@ -331,11 +336,6 @@ cleanup()
cd ${CEPH_ROOT}
CEPH_ARGS='' ${CEPH_SRC}/mstop.sh ${CLUSTER1}
CEPH_ARGS='' ${CEPH_SRC}/mstop.sh ${CLUSTER2}
else
CEPH_ARGS='' ceph --cluster ${CLUSTER1} osd pool rm ${POOL} ${POOL} --yes-i-really-really-mean-it
CEPH_ARGS='' ceph --cluster ${CLUSTER2} osd pool rm ${POOL} ${POOL} --yes-i-really-really-mean-it
CEPH_ARGS='' ceph --cluster ${CLUSTER1} osd pool rm ${PARENT_POOL} ${PARENT_POOL} --yes-i-really-really-mean-it
CEPH_ARGS='' ceph --cluster ${CLUSTER2} osd pool rm ${PARENT_POOL} ${PARENT_POOL} --yes-i-really-really-mean-it
fi
test "${RBD_MIRROR_TEMDIR}" = "${TEMPDIR}" || rm -Rf ${TEMPDIR}
fi

View File

@ -454,9 +454,17 @@ template <typename I>
bool ManagedLock<I>::is_state_shutdown() const {
ceph_assert(m_lock.is_locked());
return ((m_state == STATE_SHUTDOWN) ||
(!m_actions_contexts.empty() &&
m_actions_contexts.back().first == ACTION_SHUT_DOWN));
switch (m_state) {
case STATE_PRE_SHUTTING_DOWN:
case STATE_SHUTTING_DOWN:
case STATE_SHUTDOWN:
return true;
default:
break;
}
return (!m_actions_contexts.empty() &&
m_actions_contexts.back().first == ACTION_SHUT_DOWN);
}
template <typename I>

View File

@ -291,6 +291,44 @@ TEST_F(TestMockImageDeleterTrashWatcher, Notify) {
ASSERT_EQ(0, when_shut_down(mock_trash_watcher));
}
TEST_F(TestMockImageDeleterTrashWatcher, CreateBlacklist) {
MockThreads mock_threads(m_threads);
expect_work_queue(mock_threads);
InSequence seq;
expect_create_trash(m_local_io_ctx, -EBLACKLISTED);
MockListener mock_listener;
MockTrashWatcher mock_trash_watcher(m_local_io_ctx, &mock_threads,
mock_listener);
C_SaferCond ctx;
mock_trash_watcher.init(&ctx);
ASSERT_EQ(-EBLACKLISTED, ctx.wait());
MockLibrbdTrashWatcher mock_librbd_trash_watcher;
expect_trash_watcher_unregister(mock_librbd_trash_watcher, 0);
ASSERT_EQ(0, when_shut_down(mock_trash_watcher));
}
TEST_F(TestMockImageDeleterTrashWatcher, CreateDNE) {
MockThreads mock_threads(m_threads);
expect_work_queue(mock_threads);
InSequence seq;
expect_create_trash(m_local_io_ctx, -ENOENT);
MockListener mock_listener;
MockTrashWatcher mock_trash_watcher(m_local_io_ctx, &mock_threads,
mock_listener);
C_SaferCond ctx;
mock_trash_watcher.init(&ctx);
ASSERT_EQ(-ENOENT, ctx.wait());
MockLibrbdTrashWatcher mock_librbd_trash_watcher;
expect_trash_watcher_unregister(mock_librbd_trash_watcher, 0);
ASSERT_EQ(0, when_shut_down(mock_trash_watcher));
}
TEST_F(TestMockImageDeleterTrashWatcher, CreateError) {
MockThreads mock_threads(m_threads);
expect_work_queue(mock_threads);

View File

@ -850,7 +850,6 @@ TEST_F(TestMockImageReplayer, GetRemoteImageIdError) {
}
TEST_F(TestMockImageReplayer, BootstrapError) {
create_local_image();
librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx);
@ -885,6 +884,42 @@ TEST_F(TestMockImageReplayer, BootstrapError) {
ASSERT_EQ(-EINVAL, start_ctx.wait());
}
TEST_F(TestMockImageReplayer, StopBeforeBootstrap) {
create_local_image();
librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx);
journal::MockJournaler mock_remote_journaler;
MockThreads mock_threads(m_threads);
expect_work_queue_repeatedly(mock_threads);
expect_add_event_after_repeatedly(mock_threads);
MockImageDeleter mock_image_deleter;
MockPrepareLocalImageRequest mock_prepare_local_image_request;
MockPrepareRemoteImageRequest mock_prepare_remote_image_request;
MockReplayStatusFormatter mock_replay_status_formatter;
expect_get_or_send_update(mock_replay_status_formatter);
InSequence seq;
expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id,
mock_local_image_ctx.name, "remote mirror uuid", 0);
expect_send(mock_prepare_remote_image_request, "remote mirror uuid",
m_remote_image_ctx->id, 0);
EXPECT_CALL(mock_remote_journaler, construct())
.WillOnce(Invoke([this]() {
m_image_replayer->stop(nullptr, true);
}));
EXPECT_CALL(mock_remote_journaler, remove_listener(_));
expect_shut_down(mock_remote_journaler, 0);
create_image_replayer(mock_threads);
C_SaferCond start_ctx;
m_image_replayer->start(&start_ctx);
ASSERT_EQ(-ECANCELED, start_ctx.wait());
}
TEST_F(TestMockImageReplayer, StartExternalReplayError) {
// START

View File

@ -498,7 +498,7 @@ TEST_F(TestMockPoolWatcher, RegisterWatcherMissing) {
mock_listener);
C_SaferCond ctx;
mock_pool_watcher.init(&ctx);
ASSERT_EQ(0, ctx.wait());
ASSERT_EQ(-ENOENT, ctx.wait());
ASSERT_TRUE(wait_for_update(1));
expect_mirroring_watcher_unregister(mock_mirroring_watcher, 0);

View File

@ -334,7 +334,7 @@ void ImageReplayer<I>::add_peer(const std::string &peer_uuid,
template <typename I>
void ImageReplayer<I>::set_state_description(int r, const std::string &desc) {
dout(20) << r << " " << desc << dendl;
dout(10) << r << " " << desc << dendl;
Mutex::Locker l(m_lock);
m_last_r = r;
@ -344,7 +344,7 @@ void ImageReplayer<I>::set_state_description(int r, const std::string &desc) {
template <typename I>
void ImageReplayer<I>::start(Context *on_finish, bool manual)
{
dout(20) << "on_finish=" << on_finish << dendl;
dout(10) << "on_finish=" << on_finish << dendl;
int r = 0;
{
@ -498,18 +498,21 @@ void ImageReplayer<I>::bootstrap() {
return;
}
Context *ctx = create_context_callback<
ImageReplayer, &ImageReplayer<I>::handle_bootstrap>(this);
BootstrapRequest<I> *request = BootstrapRequest<I>::create(
m_threads, *m_local_ioctx, m_remote_image.io_ctx, m_instance_watcher,
&m_local_image_ctx, m_local_image_id, m_remote_image.image_id,
m_global_image_id, m_local_mirror_uuid, m_remote_image.mirror_uuid,
m_remote_journaler, &m_client_state, &m_client_meta, ctx,
&m_resync_requested, &m_progress_cxt);
BootstrapRequest<I> *request = nullptr;
{
Mutex::Locker locker(m_lock);
if (on_start_interrupted(m_lock)) {
return;
}
auto ctx = create_context_callback<
ImageReplayer, &ImageReplayer<I>::handle_bootstrap>(this);
request = BootstrapRequest<I>::create(
m_threads, *m_local_ioctx, m_remote_image.io_ctx, m_instance_watcher,
&m_local_image_ctx, m_local_image_id, m_remote_image.image_id,
m_global_image_id, m_local_mirror_uuid, m_remote_image.mirror_uuid,
m_remote_journaler, &m_client_state, &m_client_meta, ctx,
&m_resync_requested, &m_progress_cxt);
request->get();
m_bootstrap_request = request;
}
@ -674,9 +677,9 @@ void ImageReplayer<I>::handle_start_replay(int r) {
dout(10) << "m_remote_journaler=" << *m_remote_journaler << dendl;
}
dout(20) << "start succeeded" << dendl;
dout(10) << "start succeeded" << dendl;
if (on_finish != nullptr) {
dout(20) << "on finish complete, r=" << r << dendl;
dout(10) << "on finish complete, r=" << r << dendl;
on_finish->complete(r);
}
}
@ -684,7 +687,7 @@ void ImageReplayer<I>::handle_start_replay(int r) {
template <typename I>
void ImageReplayer<I>::on_start_fail(int r, const std::string &desc)
{
dout(20) << "r=" << r << dendl;
dout(10) << "r=" << r << dendl;
Context *ctx = new FunctionContext([this, r, desc](int _r) {
{
Mutex::Locker locker(m_lock);
@ -693,7 +696,7 @@ void ImageReplayer<I>::on_start_fail(int r, const std::string &desc)
if (r < 0 && r != -ECANCELED && r != -EREMOTEIO && r != -ENOENT) {
derr << "start failed: " << cpp_strerror(r) << dendl;
} else {
dout(20) << "start canceled" << dendl;
dout(10) << "start canceled" << dendl;
}
}
@ -708,11 +711,16 @@ void ImageReplayer<I>::on_start_fail(int r, const std::string &desc)
}
template <typename I>
bool ImageReplayer<I>::on_start_interrupted()
{
bool ImageReplayer<I>::on_start_interrupted() {
Mutex::Locker locker(m_lock);
return on_start_interrupted(m_lock);
}
template <typename I>
bool ImageReplayer<I>::on_start_interrupted(Mutex& lock) {
ceph_assert(m_lock.is_locked());
ceph_assert(m_state == STATE_STARTING);
if (m_on_stop_finish == nullptr) {
if (!m_stop_requested) {
return false;
}
@ -724,7 +732,7 @@ template <typename I>
void ImageReplayer<I>::stop(Context *on_finish, bool manual, int r,
const std::string& desc)
{
dout(20) << "on_finish=" << on_finish << ", manual=" << manual
dout(10) << "on_finish=" << on_finish << ", manual=" << manual
<< ", desc=" << desc << dendl;
image_replayer::BootstrapRequest<I> *bootstrap_request = nullptr;
@ -738,13 +746,13 @@ void ImageReplayer<I>::stop(Context *on_finish, bool manual, int r,
} else {
if (!is_stopped_()) {
if (m_state == STATE_STARTING) {
dout(20) << "canceling start" << dendl;
if (m_bootstrap_request) {
dout(10) << "canceling start" << dendl;
if (m_bootstrap_request != nullptr) {
bootstrap_request = m_bootstrap_request;
bootstrap_request->get();
}
} else {
dout(20) << "interrupting replay" << dendl;
dout(10) << "interrupting replay" << dendl;
shut_down_replay = true;
}
@ -758,6 +766,7 @@ void ImageReplayer<I>::stop(Context *on_finish, bool manual, int r,
// avoid holding lock since bootstrap request will update status
if (bootstrap_request != nullptr) {
dout(10) << "canceling bootstrap" << dendl;
bootstrap_request->cancel();
bootstrap_request->put();
}
@ -780,7 +789,7 @@ void ImageReplayer<I>::stop(Context *on_finish, bool manual, int r,
template <typename I>
void ImageReplayer<I>::on_stop_journal_replay(int r, const std::string &desc)
{
dout(20) << "enter" << dendl;
dout(10) << dendl;
{
Mutex::Locker locker(m_lock);
@ -801,7 +810,7 @@ void ImageReplayer<I>::on_stop_journal_replay(int r, const std::string &desc)
template <typename I>
void ImageReplayer<I>::handle_replay_ready()
{
dout(20) << "enter" << dendl;
dout(20) << dendl;
if (on_replay_interrupted()) {
return;
}
@ -846,7 +855,7 @@ void ImageReplayer<I>::restart(Context *on_finish)
template <typename I>
void ImageReplayer<I>::flush(Context *on_finish)
{
dout(20) << "enter" << dendl;
dout(10) << dendl;
{
Mutex::Locker locker(m_lock);
@ -870,7 +879,7 @@ void ImageReplayer<I>::flush(Context *on_finish)
template <typename I>
void ImageReplayer<I>::on_flush_local_replay_flush_start(Context *on_flush)
{
dout(20) << "enter" << dendl;
dout(10) << dendl;
FunctionContext *ctx = new FunctionContext(
[this, on_flush](int r) {
on_flush_local_replay_flush_finish(on_flush, r);
@ -885,7 +894,7 @@ template <typename I>
void ImageReplayer<I>::on_flush_local_replay_flush_finish(Context *on_flush,
int r)
{
dout(20) << "r=" << r << dendl;
dout(10) << "r=" << r << dendl;
if (r < 0) {
derr << "error flushing local replay: " << cpp_strerror(r) << dendl;
on_flush->complete(r);
@ -939,7 +948,7 @@ bool ImageReplayer<I>::on_replay_interrupted()
template <typename I>
void ImageReplayer<I>::print_status(Formatter *f, stringstream *ss)
{
dout(20) << "enter" << dendl;
dout(10) << dendl;
Mutex::Locker l(m_lock);
@ -957,7 +966,7 @@ void ImageReplayer<I>::print_status(Formatter *f, stringstream *ss)
template <typename I>
void ImageReplayer<I>::handle_replay_complete(int r, const std::string &error_desc)
{
dout(20) << "r=" << r << dendl;
dout(10) << "r=" << r << dendl;
if (r < 0) {
derr << "replay encountered an error: " << cpp_strerror(r) << dendl;
set_state_description(r, error_desc);
@ -972,13 +981,13 @@ void ImageReplayer<I>::handle_replay_complete(int r, const std::string &error_de
template <typename I>
void ImageReplayer<I>::replay_flush() {
dout(20) << dendl;
dout(10) << dendl;
bool interrupted = false;
{
Mutex::Locker locker(m_lock);
if (m_state != STATE_REPLAYING) {
dout(20) << "replay interrupted" << dendl;
dout(10) << "replay interrupted" << dendl;
interrupted = true;
} else {
m_state = STATE_REPLAY_FLUSHING;
@ -1010,7 +1019,7 @@ void ImageReplayer<I>::replay_flush() {
template <typename I>
void ImageReplayer<I>::handle_replay_flush(int r) {
dout(20) << "r=" << r << dendl;
dout(10) << "r=" << r << dendl;
{
Mutex::Locker locker(m_lock);
@ -1279,7 +1288,7 @@ void ImageReplayer<I>::handle_process_entry_safe(const ReplayEntry &replay_entry
template <typename I>
bool ImageReplayer<I>::update_mirror_image_status(bool force,
const OptionalState &state) {
dout(20) << dendl;
dout(15) << dendl;
{
Mutex::Locker locker(m_lock);
if (!start_mirror_image_status_update(force, false)) {
@ -1298,10 +1307,10 @@ bool ImageReplayer<I>::start_mirror_image_status_update(bool force,
if (!force && !is_stopped_()) {
if (!is_running_()) {
dout(20) << "shut down in-progress: ignoring update" << dendl;
dout(15) << "shut down in-progress: ignoring update" << dendl;
return false;
} else if (m_in_flight_status_updates > (restarting ? 1 : 0)) {
dout(20) << "already sending update" << dendl;
dout(15) << "already sending update" << dendl;
m_update_status_requested = true;
return false;
}
@ -1337,7 +1346,7 @@ void ImageReplayer<I>::finish_mirror_image_status_update() {
template <typename I>
void ImageReplayer<I>::queue_mirror_image_status_update(const OptionalState &state) {
dout(20) << dendl;
dout(15) << dendl;
FunctionContext *ctx = new FunctionContext(
[this, state](int r) {
send_mirror_status_update(state);
@ -1399,7 +1408,7 @@ void ImageReplayer<I>::send_mirror_status_update(const OptionalState &opt_state)
{
Context *on_req_finish = new FunctionContext(
[this](int r) {
dout(20) << "replay status ready: r=" << r << dendl;
dout(15) << "replay status ready: r=" << r << dendl;
if (r >= 0) {
send_mirror_status_update(boost::none);
} else if (r == -EAGAIN) {
@ -1412,7 +1421,7 @@ void ImageReplayer<I>::send_mirror_status_update(const OptionalState &opt_state)
ceph_assert(m_replay_status_formatter != nullptr);
if (!m_replay_status_formatter->get_or_send_update(&desc,
on_req_finish)) {
dout(20) << "waiting for replay status" << dendl;
dout(15) << "waiting for replay status" << dendl;
return;
}
status.description = "replaying, " + desc;
@ -1567,7 +1576,7 @@ void ImageReplayer<I>::shut_down(int r) {
// before proceeding
if (m_in_flight_status_updates > 0) {
if (m_on_update_status_finish == nullptr) {
dout(20) << "waiting for in-flight status update" << dendl;
dout(15) << "waiting for in-flight status update" << dendl;
m_on_update_status_finish = new FunctionContext(
[this, r](int _r) {
shut_down(r);
@ -1682,7 +1691,7 @@ void ImageReplayer<I>::handle_shut_down(int r) {
// before proceeding
if (m_in_flight_status_updates > 0) {
if (m_on_update_status_finish == nullptr) {
dout(20) << "waiting for in-flight status update" << dendl;
dout(15) << "waiting for in-flight status update" << dendl;
m_on_update_status_finish = new FunctionContext(
[this, r](int _r) {
handle_shut_down(r);
@ -1740,12 +1749,12 @@ void ImageReplayer<I>::handle_shut_down(int r) {
}
if (on_start != nullptr) {
dout(20) << "on start finish complete, r=" << r << dendl;
dout(10) << "on start finish complete, r=" << r << dendl;
on_start->complete(r);
r = 0;
}
if (on_stop != nullptr) {
dout(20) << "on stop finish complete, r=" << r << dendl;
dout(10) << "on stop finish complete, r=" << r << dendl;
on_stop->complete(r);
}
}
@ -1795,7 +1804,7 @@ std::string ImageReplayer<I>::to_string(const State state) {
template <typename I>
void ImageReplayer<I>::resync_image(Context *on_finish) {
dout(20) << dendl;
dout(10) << dendl;
m_resync_requested = true;
stop(on_finish);
@ -1861,7 +1870,7 @@ void ImageReplayer<I>::reregister_admin_socket_hook() {
{
Mutex::Locker locker(m_lock);
auto name = m_local_ioctx->get_pool_name() + "/" + m_local_image_name;
if (m_name == name) {
if (m_asok_hook != nullptr && m_name == name) {
return;
}
m_name = name;

View File

@ -199,6 +199,7 @@ protected:
virtual void on_start_fail(int r, const std::string &desc);
virtual bool on_start_interrupted();
virtual bool on_start_interrupted(Mutex& lock);
virtual void on_stop_journal_replay(int r = 0, const std::string &desc = "");

View File

@ -902,8 +902,10 @@ void LeaderWatcher<I>::handle_notify_lock_acquired(int r) {
ceph_assert(m_on_finish != nullptr);
std::swap(m_on_finish, on_finish);
// listener should be ready for instance add/remove events now
m_instances->unblock_listener();
if (m_ret_val == 0) {
// listener should be ready for instance add/remove events now
m_instances->unblock_listener();
}
}
on_finish->complete(0);
}
@ -1090,7 +1092,9 @@ template <typename I>
void LeaderWatcher<I>::handle_rewatch_complete(int r) {
dout(5) << "r=" << r << dendl;
m_leader_lock->reacquire_lock(nullptr);
if (r != -EBLACKLISTED) {
m_leader_lock->reacquire_lock(nullptr);
}
}
template <typename I>

View File

@ -849,7 +849,13 @@ template <typename I>
void PoolReplayer<I>::handle_init_remote_pool_watcher(
int r, Context *on_finish) {
dout(10) << "r=" << r << dendl;
if (r < 0) {
if (r == -ENOENT) {
// Technically nothing to do since the other side doesn't
// have mirroring enabled. Eventually the remote pool watcher will
// detect images (if mirroring is enabled), so no point propagating
// an error which would just busy-spin the state machines.
dout(0) << "remote peer does not have mirroring configured" << dendl;
} else if (r < 0) {
derr << "failed to retrieve remote images: " << cpp_strerror(r) << dendl;
on_finish = new FunctionContext([on_finish, r](int) {
on_finish->complete(r);

View File

@ -174,6 +174,9 @@ void PoolWatcher<I>::handle_register_watcher(int r) {
} else if (r == -ENOENT) {
dout(5) << "mirroring directory does not exist" << dendl;
schedule_refresh_images(30);
Mutex::Locker locker(m_lock);
std::swap(on_init_finish, m_on_init_finish);
} else {
derr << "unexpected error registering mirroring directory watch: "
<< cpp_strerror(r) << dendl;

View File

@ -134,7 +134,18 @@ void TrashWatcher<I>::handle_create_trash(int r) {
ceph_assert(m_trash_list_in_progress);
}
if (r < 0 && r != -EEXIST) {
Context* on_init_finish = nullptr;
if (r == -EBLACKLISTED || r == -ENOENT) {
if (r == -EBLACKLISTED) {
dout(0) << "detected client is blacklisted" << dendl;
} else {
dout(0) << "detected pool no longer exists" << dendl;
}
Mutex::Locker locker(m_lock);
std::swap(on_init_finish, m_on_init_finish);
m_trash_list_in_progress = false;
} else if (r < 0 && r != -EEXIST) {
derr << "failed to create trash object: " << cpp_strerror(r) << dendl;
{
Mutex::Locker locker(m_lock);
@ -147,6 +158,9 @@ void TrashWatcher<I>::handle_create_trash(int r) {
}
m_async_op_tracker.finish_op();
if (on_init_finish != nullptr) {
on_init_finish->complete(r);
}
}
template <typename I>

View File

@ -96,9 +96,9 @@ int main(int argc, const char **argv)
shutdown_async_signal_handler();
g_ceph_context->get_perfcounters_collection()->remove(g_perf_counters);
delete g_perf_counters;
delete mirror;
delete g_perf_counters;
return r < 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}