Skip to content

Commit

Permalink
librbd: separate break lock logic into standalone state machine
Browse files Browse the repository at this point in the history 
The current lockers are now queried before the lock is attempted to
prevent any possible race conditions when one or more clients attempt
to break the lock of a dead client.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit 23f60fe)
  • Loading branch information
Jason Dillaman committed Jan 11, 2017
1 parent b2e9ad8 commit 9b9eef8
Show file tree
Hide file tree
Showing 10 changed files with 655 additions and 368 deletions.
1 change: 1 addition & 0 deletions src/librbd/CMakeLists.txt
View file
Expand Up @@ -28,6 +28,7 @@ set(librbd_internal_srcs
Utils.cc
exclusive_lock/AcquireRequest.cc
exclusive_lock/AutomaticPolicy.cc
exclusive_lock/BreakRequest.cc
exclusive_lock/GetLockerRequest.cc
exclusive_lock/ReacquireRequest.cc
exclusive_lock/ReleaseRequest.cc
Expand Down
2 changes: 2 additions & 0 deletions src/librbd/Makefile.am
View file
Expand Up @@ -33,6 +33,7 @@ librbd_internal_la_SOURCES = \
librbd/Utils.cc \
librbd/exclusive_lock/AcquireRequest.cc \
librbd/exclusive_lock/AutomaticPolicy.cc \
librbd/exclusive_lock/BreakRequest.cc \
librbd/exclusive_lock/GetLockerRequest.cc \
librbd/exclusive_lock/ReacquireRequest.cc \
librbd/exclusive_lock/ReleaseRequest.cc \
Expand Down Expand Up @@ -122,6 +123,7 @@ noinst_HEADERS += \
librbd/WatchNotifyTypes.h \
librbd/exclusive_lock/AcquireRequest.h \
librbd/exclusive_lock/AutomaticPolicy.h \
librbd/exclusive_lock/BreakRequest.h \
librbd/exclusive_lock/GetLockerRequest.h \
librbd/exclusive_lock/Policy.h \
librbd/exclusive_lock/ReacquireRequest.h \
Expand Down
200 changes: 48 additions & 152 deletions src/librbd/exclusive_lock/AcquireRequest.cc
View file
Expand Up @@ -15,6 +15,7 @@
#include "librbd/Journal.h"
#include "librbd/ObjectMap.h"
#include "librbd/Utils.h"
#include "librbd/exclusive_lock/BreakRequest.h"
#include "librbd/exclusive_lock/GetLockerRequest.h"
#include "librbd/image/RefreshRequest.h"
#include "librbd/journal/Policy.h"
Expand All @@ -31,30 +32,6 @@ using util::create_context_callback;
using util::create_rados_ack_callback;
using util::create_rados_safe_callback;

namespace {

template <typename I>
struct C_BlacklistClient : public Context {
I &image_ctx;
std::string locker_address;
Context *on_finish;

C_BlacklistClient(I &image_ctx, const std::string &locker_address,
Context *on_finish)
: image_ctx(image_ctx), locker_address(locker_address),
on_finish(on_finish) {
}

virtual void finish(int r) override {
librados::Rados rados(image_ctx.md_ctx);
r = rados.blacklist_add(locker_address,
image_ctx.blacklist_expire_seconds);
on_finish->complete(r);
}
};

} // anonymous namespace

template <typename I>
AcquireRequest<I>* AcquireRequest<I>::create(I &image_ctx,
const std::string &cookie,
Expand Down Expand Up @@ -121,6 +98,39 @@ Context *AcquireRequest<I>::handle_flush_notifies(int *ret_val) {
ldout(cct, 10) << __func__ << dendl;

assert(*ret_val == 0);
send_get_locker();
return nullptr;
}

template <typename I>
void AcquireRequest<I>::send_get_locker() {
CephContext *cct = m_image_ctx.cct;
ldout(cct, 10) << __func__ << dendl;

Context *ctx = create_context_callback<
AcquireRequest<I>, &AcquireRequest<I>::handle_get_locker>(this);
auto req = GetLockerRequest<I>::create(m_image_ctx, &m_locker, ctx);
req->send();
}

template <typename I>
Context *AcquireRequest<I>::handle_get_locker(int *ret_val) {
CephContext *cct = m_image_ctx.cct;
ldout(cct, 10) << __func__ << ": r=" << *ret_val << dendl;

if (*ret_val == -ENOENT) {
ldout(cct, 20) << "no lockers detected" << dendl;
m_locker = {};
*ret_val = 0;
} else if (*ret_val == -EBUSY) {
ldout(cct, 5) << "incompatible lock detected" << dendl;
return m_on_finish;
} else if (*ret_val < 0) {
lderr(cct) << "failed to retrieve lockers: " << cpp_strerror(*ret_val)
<< dendl;
return m_on_finish;
}

send_lock();
return nullptr;
}
Expand Down Expand Up @@ -150,12 +160,16 @@ Context *AcquireRequest<I>::handle_lock(int *ret_val) {

if (*ret_val == 0) {
return send_refresh();
} else if (*ret_val == -EBUSY && m_locker.cookie.empty()) {
ldout(cct, 5) << "already locked, refreshing locker" << dendl;
send_get_locker();
return nullptr;
} else if (*ret_val != -EBUSY) {
lderr(cct) << "failed to lock: " << cpp_strerror(*ret_val) << dendl;
return m_on_finish;
}

send_get_locker();
send_break_lock();
return nullptr;
}

Expand Down Expand Up @@ -394,149 +408,31 @@ Context *AcquireRequest<I>::handle_unlock(int *ret_val) {
}

template <typename I>
void AcquireRequest<I>::send_get_locker() {
void AcquireRequest<I>::send_break_lock() {
CephContext *cct = m_image_ctx.cct;
ldout(cct, 10) << __func__ << dendl;

Context *ctx = create_context_callback<
AcquireRequest<I>, &AcquireRequest<I>::handle_get_locker>(this);
auto req = GetLockerRequest<I>::create(m_image_ctx, &m_locker, ctx);
AcquireRequest<I>, &AcquireRequest<I>::handle_break_lock>(this);
auto req = BreakRequest<I>::create(
m_image_ctx, m_locker, m_image_ctx.blacklist_on_break_lock, false, ctx);
req->send();
}

template <typename I>
Context *AcquireRequest<I>::handle_get_locker(int *ret_val) {
CephContext *cct = m_image_ctx.cct;
ldout(cct, 10) << __func__ << ": r=" << *ret_val << dendl;

if (*ret_val == -ENOENT) {
ldout(cct, 20) << "no lockers detected" << dendl;
send_lock();
return nullptr;
} else if (*ret_val == -EBUSY) {
ldout(cct, 5) << "incompatible lock detected" << dendl;
return m_on_finish;
} else if (*ret_val < 0) {
lderr(cct) << "failed to retrieve lockers: " << cpp_strerror(*ret_val)
<< dendl;
return m_on_finish;
}

send_get_watchers();
return nullptr;
}

template <typename I>
void AcquireRequest<I>::send_get_watchers() {
CephContext *cct = m_image_ctx.cct;
ldout(cct, 10) << __func__ << dendl;

librados::ObjectReadOperation op;
op.list_watchers(&m_watchers, &m_watchers_ret_val);

using klass = AcquireRequest<I>;
librados::AioCompletion *rados_completion =
create_rados_ack_callback<klass, &klass::handle_get_watchers>(this);
m_out_bl.clear();
int r = m_image_ctx.md_ctx.aio_operate(m_image_ctx.header_oid,
rados_completion, &op, &m_out_bl);
assert(r == 0);
rados_completion->release();
}

template <typename I>
Context *AcquireRequest<I>::handle_get_watchers(int *ret_val) {
CephContext *cct = m_image_ctx.cct;
ldout(cct, 10) << __func__ << ": r=" << *ret_val << dendl;

if (*ret_val == 0) {
*ret_val = m_watchers_ret_val;
}
if (*ret_val < 0) {
lderr(cct) << "failed to retrieve watchers: " << cpp_strerror(*ret_val)
<< dendl;
return m_on_finish;
}

for (auto &watcher : m_watchers) {
if ((strncmp(m_locker.address.c_str(),
watcher.addr, sizeof(watcher.addr)) == 0) &&
(m_locker.handle == watcher.cookie)) {
ldout(cct, 10) << "lock owner is still alive" << dendl;

*ret_val = -EAGAIN;
return m_on_finish;
}
}

send_blacklist();
return nullptr;
}

template <typename I>
void AcquireRequest<I>::send_blacklist() {
if (!m_image_ctx.blacklist_on_break_lock) {
send_break_lock();
return;
}

CephContext *cct = m_image_ctx.cct;
ldout(cct, 10) << __func__ << dendl;

// TODO: need async version of RadosClient::blacklist_add
using klass = AcquireRequest<I>;
Context *ctx = create_context_callback<klass, &klass::handle_blacklist>(
this);
m_image_ctx.op_work_queue->queue(new C_BlacklistClient<I>(m_image_ctx,
m_locker.address,
ctx), 0);
}

template <typename I>
Context *AcquireRequest<I>::handle_blacklist(int *ret_val) {
CephContext *cct = m_image_ctx.cct;
ldout(cct, 10) << __func__ << ": r=" << *ret_val << dendl;

if (*ret_val < 0) {
lderr(cct) << "failed to blacklist lock owner: " << cpp_strerror(*ret_val)
<< dendl;
return m_on_finish;
}
send_break_lock();
return nullptr;
}

template <typename I>
void AcquireRequest<I>::send_break_lock() {
CephContext *cct = m_image_ctx.cct;
ldout(cct, 10) << __func__ << dendl;

librados::ObjectWriteOperation op;
rados::cls::lock::break_lock(&op, RBD_LOCK_NAME, m_locker.cookie,
m_locker.entity);

using klass = AcquireRequest<I>;
librados::AioCompletion *rados_completion =
create_rados_safe_callback<klass, &klass::handle_break_lock>(this);
int r = m_image_ctx.md_ctx.aio_operate(m_image_ctx.header_oid,
rados_completion, &op);
assert(r == 0);
rados_completion->release();
}

template <typename I>
Context *AcquireRequest<I>::handle_break_lock(int *ret_val) {
CephContext *cct = m_image_ctx.cct;
ldout(cct, 10) << __func__ << ": r=" << *ret_val << dendl;

if (*ret_val == -ENOENT) {
*ret_val = 0;
if (*ret_val == -EAGAIN) {
ldout(cct, 5) << "lock owner is still alive" << dendl;
return m_on_finish;
} else if (*ret_val < 0) {
lderr(cct) << "failed to break lock: " << cpp_strerror(*ret_val) << dendl;
lderr(cct) << "failed to break lock : " << cpp_strerror(*ret_val) << dendl;
return m_on_finish;
}

send_lock();
send_get_locker();
return nullptr;
}

Expand Down
53 changes: 21 additions & 32 deletions src/librbd/exclusive_lock/AcquireRequest.h
View file
Expand Up @@ -40,24 +40,24 @@ class AcquireRequest {
* v
* FLUSH_NOTIFIES
* |
* | /-----------------------------------------------------------\
* | | |
* | | (no lockers) |
* | | . . . . . . . . . . . . . . . . . . . . . . |
* | | . . |
* | v v (EBUSY) . |
* \--> LOCK_IMAGE * * * * * * * * > GET_LOCKERS . . . . |
* | | |
* v v |
* REFRESH (skip if not GET_WATCHERS |
* | needed) | |
* v v |
* OPEN_OBJECT_MAP (skip if BLACKLIST (skip if blacklist |
* | disabled) | disabled) |
* v v |
* OPEN_JOURNAL (skip if BREAK_LOCK |
* | * disabled) | |
* | * \-----------------------------/
* v
* GET_LOCKERS <--------------------------------------\
* | ^ |
* | . (EBUSY && no cached locker) |
* | . |
* | . (EBUSY && cached locker) |
* \--> LOCK_IMAGE * * * * * * * * > BREAK_LOCK ---/
* |
* v
* REFRESH (skip if not
* | needed)
* v
* OPEN_OBJECT_MAP (skip if
* | disabled)
* v
* OPEN_JOURNAL (skip if
* | * disabled)
* | *
* | * * * * * * * *
* v *
* ALLOCATE_JOURNAL_TAG *
Expand Down Expand Up @@ -86,11 +86,6 @@ class AcquireRequest {
Context *m_on_acquire;
Context *m_on_finish;

bufferlist m_out_bl;

std::list<obj_watch_t> m_watchers;
int m_watchers_ret_val;

decltype(m_image_ctx.object_map) m_object_map;
decltype(m_image_ctx.journal) m_journal;

Expand All @@ -105,6 +100,9 @@ class AcquireRequest {
void send_flush_notifies();
Context *handle_flush_notifies(int *ret_val);

void send_get_locker();
Context *handle_get_locker(int *ret_val);

void send_lock();
Context *handle_lock(int *ret_val);

Expand All @@ -129,15 +127,6 @@ class AcquireRequest {
void send_unlock();
Context *handle_unlock(int *ret_val);

void send_get_locker();
Context *handle_get_locker(int *ret_val);

void send_get_watchers();
Context *handle_get_watchers(int *ret_val);

void send_blacklist();
Context *handle_blacklist(int *ret_val);

void send_break_lock();
Context *handle_break_lock(int *ret_val);

Expand Down

0 comments on commit 9b9eef8

Please sign in to comment.