librbd: unlock image if journal error encountered during lock

Explicitly unlock to prevent a client from accidentally blacklisting
itself when retrying the lock.

Fixes: http://tracker.ceph.com/issues/15709
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
This commit is contained in:
Jason Dillaman 2016-05-03 10:15:08 -04:00
parent fe20133543
commit a11f5e8e55
3 changed files with 69 additions and 18 deletions

View File

@ -218,7 +218,13 @@ Context *AcquireRequest<I>::handle_close_journal(int *ret_val) {
CephContext *cct = m_image_ctx.cct;
ldout(cct, 10) << __func__ << ": r=" << *ret_val << dendl;
return send_close_object_map(ret_val);
if (*ret_val < 0) {
lderr(cct) << "failed to close journal: " << cpp_strerror(*ret_val)
<< dendl;
}
send_close_object_map();
return nullptr;
}
template <typename I>
@ -250,10 +256,10 @@ Context *AcquireRequest<I>::handle_open_object_map(int *ret_val) {
}
template <typename I>
Context *AcquireRequest<I>::send_close_object_map(int *ret_val) {
void AcquireRequest<I>::send_close_object_map() {
if (m_object_map == nullptr) {
revert(ret_val);
return m_on_finish;
send_unlock();
return;
}
CephContext *cct = m_image_ctx.cct;
@ -263,7 +269,6 @@ Context *AcquireRequest<I>::send_close_object_map(int *ret_val) {
Context *ctx = create_context_callback<
klass, &klass::handle_close_object_map>(this);
m_object_map->close(ctx);
return nullptr;
}
template <typename I>
@ -273,6 +278,36 @@ Context *AcquireRequest<I>::handle_close_object_map(int *ret_val) {
// object map should never result in an error
assert(*ret_val == 0);
send_unlock();
return nullptr;
}
template <typename I>
void AcquireRequest<I>::send_unlock() {
CephContext *cct = m_image_ctx.cct;
ldout(cct, 10) << __func__ << dendl;
librados::ObjectWriteOperation op;
rados::cls::lock::unlock(&op, RBD_LOCK_NAME, m_cookie);
using klass = AcquireRequest<I>;
librados::AioCompletion *rados_completion =
create_rados_safe_callback<klass, &klass::handle_unlock>(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_unlock(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 unlock image: " << cpp_strerror(*ret_val) << dendl;
}
revert(ret_val);
return m_on_finish;
}

View File

@ -49,21 +49,26 @@ private:
* . | | |
* . . . . | | |
* . v v |
* . OPEN_OBJECT_MAP GET_WATCHERS . . . |
* . | | . |
* . OPEN_OBJECT_MAP (skip if GET_WATCHERS . . . |
* . | disabled) | . |
* . v v . |
* . . > OPEN_JOURNAL * * * * * * BLACKLIST . (blacklist |
* . | * | . disabled) |
* . v * v . |
* . ALLOCATE_JOURNAL_TAG * BREAK_LOCK < . . . |
* . | * * | |
* . | * * \-----------------------------/
* . | v v
* . . > OPEN_JOURNAL (skip if BLACKLIST . (blacklist |
* . | * disabled) | . disabled) |
* . | * v . |
* . | * * * * * * * * BREAK_LOCK < . . . |
* . v * | |
* . ALLOCATE_JOURNAL_TAG * \-----------------------------/
* . | * *
* . | * *
* . | v v
* . | CLOSE_JOURNAL
* . | |
* . | v
* . | CLOSE_OBJECT_MAP
* . | |
* . | v
* . | UNLOCK_IMAGE
* . | |
* . v |
* . . > <finish> <----------/
*
@ -105,15 +110,18 @@ private:
void send_allocate_journal_tag();
Context *handle_allocate_journal_tag(int *ret_val);
void send_close_journal();
Context *handle_close_journal(int *ret_val);
Context *send_open_object_map();
Context *handle_open_object_map(int *ret_val);
Context *send_close_object_map(int *ret_val);
void send_close_journal();
Context *handle_close_journal(int *ret_val);
void send_close_object_map();
Context *handle_close_object_map(int *ret_val);
void send_unlock();
Context *handle_unlock(int *ret_val);
void send_get_lockers();
Context *handle_get_lockers(int *ret_val);

View File

@ -52,6 +52,12 @@ public:
.WillOnce(Return(r));
}
void expect_unlock(MockImageCtx &mock_image_ctx, int r) {
EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx),
exec(mock_image_ctx.header_oid, _, StrEq("lock"), StrEq("unlock"), _, _, _))
.WillOnce(Return(r));
}
void expect_create_object_map(MockImageCtx &mock_image_ctx,
MockObjectMap *mock_object_map) {
EXPECT_CALL(mock_image_ctx, create_object_map(_))
@ -293,6 +299,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, JournalError) {
expect_open_journal(mock_image_ctx, *mock_journal, -EINVAL);
expect_close_journal(mock_image_ctx, *mock_journal);
expect_close_object_map(mock_image_ctx, *mock_object_map);
expect_unlock(mock_image_ctx, 0);
C_SaferCond acquire_ctx;
C_SaferCond ctx;
@ -330,6 +337,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, AllocateJournalTagError) {
expect_allocate_journal_tag(mock_image_ctx, mock_journal_policy, -EPERM);
expect_close_journal(mock_image_ctx, *mock_journal);
expect_close_object_map(mock_image_ctx, *mock_object_map);
expect_unlock(mock_image_ctx, 0);
C_SaferCond acquire_ctx;
C_SaferCond ctx;