From: Jason Dillaman Date: Thu, 6 Sep 2018 21:08:12 +0000 (-0400) Subject: librbd: use the correct error code when the exclusive lock isn't locked X-Git-Tag: v13.2.3~133^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=e4e7b994bc38d170f99fc627c1c6fee5a7626edb;p=ceph.git librbd: use the correct error code when the exclusive lock isn't locked If the client is currently blacklisted, use -EBLACKLISTED, otherwise use -EROFS. Signed-off-by: Jason Dillaman (cherry picked from commit e8eee15518facf562adf1aaba02d3a9523cdd2c3) Conflicts: src/librbd/ExclusiveLock.cc: trivial resolution src/librbd/Operations.cc: trivial resolution src/librbd/deep_copy/ObjectCopyRequest.cc: trivial resolution src/librbd/deep_copy/SetHeadRequest.cc: trivial resolution src/librbd/deep_copy/SnapshotCopyRequest.cc: trivial resolution src/librbd/deep_copy/SnapshotCreateRequest.cc: trivial resolution --- diff --git a/src/librbd/DeepCopyRequest.cc b/src/librbd/DeepCopyRequest.cc index d4a20a717abb..d8bb9ff8a5f7 100644 --- a/src/librbd/DeepCopyRequest.cc +++ b/src/librbd/DeepCopyRequest.cc @@ -197,14 +197,15 @@ void DeepCopyRequest::send_copy_object_map() { ldout(m_cct, 20) << dendl; Context *finish_op_ctx = nullptr; + int r; if (m_dst_image_ctx->exclusive_lock != nullptr) { - finish_op_ctx = m_dst_image_ctx->exclusive_lock->start_op(); + finish_op_ctx = m_dst_image_ctx->exclusive_lock->start_op(&r); } if (finish_op_ctx == nullptr) { lderr(m_cct) << "lost exclusive lock" << dendl; m_dst_image_ctx->snap_lock.put_read(); m_dst_image_ctx->owner_lock.put_read(); - finish(-EROFS); + finish(r); return; } @@ -231,16 +232,17 @@ void DeepCopyRequest::handle_copy_object_map(int r) { template void DeepCopyRequest::send_refresh_object_map() { + int r; Context *finish_op_ctx = nullptr; { RWLock::RLocker owner_locker(m_dst_image_ctx->owner_lock); if (m_dst_image_ctx->exclusive_lock != nullptr) { - finish_op_ctx = m_dst_image_ctx->exclusive_lock->start_op(); + finish_op_ctx = m_dst_image_ctx->exclusive_lock->start_op(&r); } } if (finish_op_ctx == nullptr) { lderr(m_cct) << "lost exclusive lock" << dendl; - finish(-EROFS); + finish(r); return; } diff --git a/src/librbd/ExclusiveLock.cc b/src/librbd/ExclusiveLock.cc index b224a7504b43..a70e77e917c0 100644 --- a/src/librbd/ExclusiveLock.cc +++ b/src/librbd/ExclusiveLock.cc @@ -137,11 +137,12 @@ void ExclusiveLock::handle_peer_notification(int r) { } template -Context *ExclusiveLock::start_op() { +Context *ExclusiveLock::start_op(int* ret_val) { assert(m_image_ctx.owner_lock.is_locked()); Mutex::Locker locker(ML::m_lock); if (!accept_ops(ML::m_lock)) { + *ret_val = get_unlocked_op_error(); return nullptr; } diff --git a/src/librbd/ExclusiveLock.h b/src/librbd/ExclusiveLock.h index 3c5ebab33a38..8b2a411f4423 100644 --- a/src/librbd/ExclusiveLock.h +++ b/src/librbd/ExclusiveLock.h @@ -24,14 +24,13 @@ public: void block_requests(int r); void unblock_requests(); - int get_unlocked_op_error() const; - void init(uint64_t features, Context *on_init); void shut_down(Context *on_shutdown); void handle_peer_notification(int r); - Context *start_op(); + int get_unlocked_op_error() const; + Context *start_op(int* ret_val); protected: void shutdown_handler(int r, Context *on_finish) override; diff --git a/src/librbd/Operations.cc b/src/librbd/Operations.cc index 67248e2c972f..8c467b61f093 100644 --- a/src/librbd/Operations.cc +++ b/src/librbd/Operations.cc @@ -226,14 +226,15 @@ struct C_InvokeAsyncRequest : public Context { ldout(cct, 20) << __func__ << ": r=" << r << dendl; if (r < 0) { - complete(-EROFS); + complete(r == -EBLACKLISTED ? -EBLACKLISTED : -EROFS); return; } // context can complete before owner_lock is unlocked RWLock &owner_lock(image_ctx.owner_lock); owner_lock.get_read(); - if (image_ctx.exclusive_lock->is_lock_owner()) { + if (image_ctx.exclusive_lock == nullptr || + image_ctx.exclusive_lock->is_lock_owner()) { send_local_request(); owner_lock.put_read(); return; @@ -799,7 +800,7 @@ int Operations::snap_rollback(const cls::rbd::SnapshotNamespace& snap_namespa r = prepare_image_update(false); if (r < 0) { - return -EROFS; + return r; } execute_snap_rollback(snap_namespace, snap_name, prog_ctx, &cond_ctx); @@ -1361,7 +1362,7 @@ int Operations::update_features(uint64_t features, bool enabled) { RWLock::RLocker owner_lock(m_image_ctx.owner_lock); r = prepare_image_update(true); if (r < 0) { - return -EROFS; + return r; } execute_update_features(features, enabled, &cond_ctx, 0); @@ -1483,10 +1484,6 @@ int Operations::metadata_remove(const std::string &key) { CephContext *cct = m_image_ctx.cct; ldout(cct, 5) << this << " " << __func__ << ": key=" << key << dendl; - if (m_image_ctx.read_only) { - return -EROFS; - } - int r = m_image_ctx.state->refresh_if_required(); if (r < 0) { return r; @@ -1582,11 +1579,14 @@ int Operations::prepare_image_update(bool request_lock) { m_image_ctx.exclusive_lock->unblock_requests(); } + if (r == -EAGAIN || r == -EBUSY) { + r = 0; + } if (r < 0) { return r; } else if (m_image_ctx.exclusive_lock != nullptr && !m_image_ctx.exclusive_lock->is_lock_owner()) { - return -EROFS; + return m_image_ctx.exclusive_lock->get_unlocked_op_error(); } return 0; diff --git a/src/librbd/deep_copy/ObjectCopyRequest.cc b/src/librbd/deep_copy/ObjectCopyRequest.cc index 19072aee7eb1..9111be5cd895 100644 --- a/src/librbd/deep_copy/ObjectCopyRequest.cc +++ b/src/librbd/deep_copy/ObjectCopyRequest.cc @@ -371,14 +371,15 @@ void ObjectCopyRequest::send_write_object() { return; } + int r; Context *finish_op_ctx; { RWLock::RLocker owner_locker(m_dst_image_ctx->owner_lock); - finish_op_ctx = start_lock_op(m_dst_image_ctx->owner_lock); + finish_op_ctx = start_lock_op(m_dst_image_ctx->owner_lock, &r); } if (finish_op_ctx == nullptr) { lderr(m_cct) << "lost exclusive lock" << dendl; - finish(-EROFS); + finish(r); return; } @@ -387,8 +388,8 @@ void ObjectCopyRequest::send_write_object() { finish_op_ctx->complete(0); }); librados::AioCompletion *comp = create_rados_callback(ctx); - int r = m_dst_io_ctx.aio_operate(m_dst_oid, comp, &op, dst_snap_seq, - dst_snap_ids); + r = m_dst_io_ctx.aio_operate(m_dst_oid, comp, &op, dst_snap_seq, + dst_snap_ids); assert(r == 0); comp->release(); } @@ -446,12 +447,13 @@ void ObjectCopyRequest::send_update_object_map() { ldout(m_cct, 20) << "dst_snap_id=" << dst_snap_id << ", object_state=" << static_cast(object_state) << dendl; - auto finish_op_ctx = start_lock_op(m_dst_image_ctx->owner_lock); + int r; + auto finish_op_ctx = start_lock_op(m_dst_image_ctx->owner_lock, &r); if (finish_op_ctx == nullptr) { lderr(m_cct) << "lost exclusive lock" << dendl; m_dst_image_ctx->snap_lock.put_read(); m_dst_image_ctx->owner_lock.put_read(); - finish(-EROFS); + finish(r); return; } @@ -486,12 +488,12 @@ void ObjectCopyRequest::handle_update_object_map(int r) { } template -Context *ObjectCopyRequest::start_lock_op(RWLock &owner_lock) { +Context *ObjectCopyRequest::start_lock_op(RWLock &owner_lock, int* r) { assert(m_dst_image_ctx->owner_lock.is_locked()); if (m_dst_image_ctx->exclusive_lock == nullptr) { return new FunctionContext([](int r) {}); } - return m_dst_image_ctx->exclusive_lock->start_op(); + return m_dst_image_ctx->exclusive_lock->start_op(r); } template diff --git a/src/librbd/deep_copy/ObjectCopyRequest.h b/src/librbd/deep_copy/ObjectCopyRequest.h index e514d539876a..2682f92b6131 100644 --- a/src/librbd/deep_copy/ObjectCopyRequest.h +++ b/src/librbd/deep_copy/ObjectCopyRequest.h @@ -179,7 +179,7 @@ private: void send_update_object_map(); void handle_update_object_map(int r); - Context *start_lock_op(RWLock &owner_lock); + Context *start_lock_op(RWLock &owner_lock, int* r); uint64_t src_to_dst_object_offset(uint64_t objectno, uint64_t offset); diff --git a/src/librbd/deep_copy/SetHeadRequest.cc b/src/librbd/deep_copy/SetHeadRequest.cc index ade87ba384d8..d82f446509bb 100644 --- a/src/librbd/deep_copy/SetHeadRequest.cc +++ b/src/librbd/deep_copy/SetHeadRequest.cc @@ -55,10 +55,11 @@ void SetHeadRequest::send_set_size() { librados::ObjectWriteOperation op; librbd::cls_client::set_size(&op, m_size); - auto finish_op_ctx = start_lock_op(); + int r; + auto finish_op_ctx = start_lock_op(&r); if (finish_op_ctx == nullptr) { lderr(m_cct) << "lost exclusive lock" << dendl; - finish(-EROFS); + finish(r); return; } @@ -67,7 +68,7 @@ void SetHeadRequest::send_set_size() { finish_op_ctx->complete(0); }); librados::AioCompletion *comp = create_rados_callback(ctx); - int r = m_image_ctx->md_ctx.aio_operate(m_image_ctx->header_oid, comp, &op); + r = m_image_ctx->md_ctx.aio_operate(m_image_ctx->header_oid, comp, &op); assert(r == 0); comp->release(); } @@ -115,10 +116,11 @@ void SetHeadRequest::send_remove_parent() { librados::ObjectWriteOperation op; librbd::cls_client::remove_parent(&op); - auto finish_op_ctx = start_lock_op(); + int r; + auto finish_op_ctx = start_lock_op(&r); if (finish_op_ctx == nullptr) { lderr(m_cct) << "lost exclusive lock" << dendl; - finish(-EROFS); + finish(r); return; } @@ -127,7 +129,7 @@ void SetHeadRequest::send_remove_parent() { finish_op_ctx->complete(0); }); librados::AioCompletion *comp = create_rados_callback(ctx); - int r = m_image_ctx->md_ctx.aio_operate(m_image_ctx->header_oid, comp, &op); + r = m_image_ctx->md_ctx.aio_operate(m_image_ctx->header_oid, comp, &op); assert(r == 0); comp->release(); } @@ -168,10 +170,11 @@ void SetHeadRequest::send_set_parent() { librados::ObjectWriteOperation op; librbd::cls_client::set_parent(&op, m_parent_spec, m_parent_overlap); - auto finish_op_ctx = start_lock_op(); + int r; + auto finish_op_ctx = start_lock_op(&r); if (finish_op_ctx == nullptr) { lderr(m_cct) << "lost exclusive lock" << dendl; - finish(-EROFS); + finish(r); return; } @@ -180,7 +183,7 @@ void SetHeadRequest::send_set_parent() { finish_op_ctx->complete(0); }); librados::AioCompletion *comp = create_rados_callback(ctx); - int r = m_image_ctx->md_ctx.aio_operate(m_image_ctx->header_oid, comp, &op); + r = m_image_ctx->md_ctx.aio_operate(m_image_ctx->header_oid, comp, &op); assert(r == 0); comp->release(); } @@ -206,12 +209,12 @@ void SetHeadRequest::handle_set_parent(int r) { } template -Context *SetHeadRequest::start_lock_op() { +Context *SetHeadRequest::start_lock_op(int* r) { RWLock::RLocker owner_locker(m_image_ctx->owner_lock); if (m_image_ctx->exclusive_lock == nullptr) { return new FunctionContext([](int r) {}); } - return m_image_ctx->exclusive_lock->start_op(); + return m_image_ctx->exclusive_lock->start_op(r); } template diff --git a/src/librbd/deep_copy/SetHeadRequest.h b/src/librbd/deep_copy/SetHeadRequest.h index 7754ff735535..e3c313f1b1c6 100644 --- a/src/librbd/deep_copy/SetHeadRequest.h +++ b/src/librbd/deep_copy/SetHeadRequest.h @@ -74,7 +74,7 @@ private: void send_set_parent(); void handle_set_parent(int r); - Context *start_lock_op(); + Context *start_lock_op(int* r); void finish(int r); }; diff --git a/src/librbd/deep_copy/SnapshotCopyRequest.cc b/src/librbd/deep_copy/SnapshotCopyRequest.cc index bc32e7e67a81..597c99bbd390 100644 --- a/src/librbd/deep_copy/SnapshotCopyRequest.cc +++ b/src/librbd/deep_copy/SnapshotCopyRequest.cc @@ -174,10 +174,11 @@ void SnapshotCopyRequest::send_snap_unprotect() { ldout(m_cct, 20) << "snap_name=" << m_snap_name << ", " << "snap_id=" << m_prev_snap_id << dendl; - auto finish_op_ctx = start_lock_op(); + int r; + auto finish_op_ctx = start_lock_op(&r); if (finish_op_ctx == nullptr) { lderr(m_cct) << "lost exclusive lock" << dendl; - finish(-EROFS); + finish(r); return; } @@ -270,10 +271,11 @@ void SnapshotCopyRequest::send_snap_remove() { << "snap_name=" << m_snap_name << ", " << "snap_id=" << m_prev_snap_id << dendl; - auto finish_op_ctx = start_lock_op(); + int r; + auto finish_op_ctx = start_lock_op(&r); if (finish_op_ctx == nullptr) { lderr(m_cct) << "lost exclusive lock" << dendl; - finish(-EROFS); + finish(r); return; } @@ -370,10 +372,11 @@ void SnapshotCopyRequest::send_snap_create() { << "snap_id=" << parent_spec.snap_id << ", " << "overlap=" << parent_overlap << "]" << dendl; - Context *finish_op_ctx = start_lock_op(); + int r; + Context *finish_op_ctx = start_lock_op(&r); if (finish_op_ctx == nullptr) { lderr(m_cct) << "lost exclusive lock" << dendl; - finish(-EROFS); + finish(r); return; } @@ -477,10 +480,11 @@ void SnapshotCopyRequest::send_snap_protect() { ldout(m_cct, 20) << "snap_name=" << m_snap_name << ", " << "snap_id=" << m_prev_snap_id << dendl; - auto finish_op_ctx = start_lock_op(); + int r; + auto finish_op_ctx = start_lock_op(&r); if (finish_op_ctx == nullptr) { lderr(m_cct) << "lost exclusive lock" << dendl; - finish(-EROFS); + finish(r); return; } @@ -571,7 +575,7 @@ void SnapshotCopyRequest::send_resize_object_map() { ldout(m_cct, 20) << dendl; - auto finish_op_ctx = start_lock_op(m_dst_image_ctx->owner_lock); + auto finish_op_ctx = start_lock_op(m_dst_image_ctx->owner_lock, &r); if (finish_op_ctx != nullptr) { auto ctx = new FunctionContext([this, finish_op_ctx](int r) { handle_resize_object_map(r); @@ -584,7 +588,6 @@ void SnapshotCopyRequest::send_resize_object_map() { } lderr(m_cct) << "lost exclusive lock" << dendl; - r = -EROFS; } } @@ -644,18 +647,18 @@ int SnapshotCopyRequest::validate_parent(I *image_ctx, } template -Context *SnapshotCopyRequest::start_lock_op() { +Context *SnapshotCopyRequest::start_lock_op(int* r) { RWLock::RLocker owner_locker(m_dst_image_ctx->owner_lock); - return start_lock_op(m_dst_image_ctx->owner_lock); + return start_lock_op(m_dst_image_ctx->owner_lock, r); } template -Context *SnapshotCopyRequest::start_lock_op(RWLock &owner_lock) { +Context *SnapshotCopyRequest::start_lock_op(RWLock &owner_lock, int* r) { assert(m_dst_image_ctx->owner_lock.is_locked()); if (m_dst_image_ctx->exclusive_lock == nullptr) { return new FunctionContext([](int r) {}); } - return m_dst_image_ctx->exclusive_lock->start_op(); + return m_dst_image_ctx->exclusive_lock->start_op(r); } template diff --git a/src/librbd/deep_copy/SnapshotCopyRequest.h b/src/librbd/deep_copy/SnapshotCopyRequest.h index 0155488b6dfc..88cf0e52c13a 100644 --- a/src/librbd/deep_copy/SnapshotCopyRequest.h +++ b/src/librbd/deep_copy/SnapshotCopyRequest.h @@ -126,8 +126,8 @@ private: int validate_parent(ImageCtxT *image_ctx, librbd::ParentSpec *spec); - Context *start_lock_op(); - Context *start_lock_op(RWLock &owner_lock); + Context *start_lock_op(int* r); + Context *start_lock_op(RWLock &owner_locki, int* r); void finish(int r); }; diff --git a/src/librbd/deep_copy/SnapshotCreateRequest.cc b/src/librbd/deep_copy/SnapshotCreateRequest.cc index a9503765ac29..f199f543e383 100644 --- a/src/librbd/deep_copy/SnapshotCreateRequest.cc +++ b/src/librbd/deep_copy/SnapshotCreateRequest.cc @@ -68,10 +68,11 @@ template void SnapshotCreateRequest::send_create_snap() { ldout(m_cct, 20) << "snap_name=" << m_snap_name << dendl; - auto finish_op_ctx = start_lock_op(); + int r; + auto finish_op_ctx = start_lock_op(&r); if (finish_op_ctx == nullptr) { lderr(m_cct) << "lost exclusive lock" << dendl; - finish(-EROFS); + finish(r); return; } @@ -131,10 +132,11 @@ void SnapshotCreateRequest::send_create_object_map() { librados::ObjectWriteOperation op; librbd::cls_client::object_map_resize(&op, object_count, OBJECT_NONEXISTENT); - auto finish_op_ctx = start_lock_op(); + int r; + auto finish_op_ctx = start_lock_op(&r); if (finish_op_ctx == nullptr) { lderr(m_cct) << "lost exclusive lock" << dendl; - finish(-EROFS); + finish(r); return; } @@ -143,7 +145,7 @@ void SnapshotCreateRequest::send_create_object_map() { finish_op_ctx->complete(0); }); librados::AioCompletion *comp = create_rados_callback(ctx); - int r = m_dst_image_ctx->md_ctx.aio_operate(object_map_oid, comp, &op); + r = m_dst_image_ctx->md_ctx.aio_operate(object_map_oid, comp, &op); assert(r == 0); comp->release(); } @@ -163,12 +165,12 @@ void SnapshotCreateRequest::handle_create_object_map(int r) { } template -Context *SnapshotCreateRequest::start_lock_op() { +Context *SnapshotCreateRequest::start_lock_op(int* r) { RWLock::RLocker owner_locker(m_dst_image_ctx->owner_lock); if (m_dst_image_ctx->exclusive_lock == nullptr) { return new FunctionContext([](int r) {}); } - return m_dst_image_ctx->exclusive_lock->start_op(); + return m_dst_image_ctx->exclusive_lock->start_op(r); } template diff --git a/src/librbd/deep_copy/SnapshotCreateRequest.h b/src/librbd/deep_copy/SnapshotCreateRequest.h index 4ddcefd19d96..a34b234bb655 100644 --- a/src/librbd/deep_copy/SnapshotCreateRequest.h +++ b/src/librbd/deep_copy/SnapshotCreateRequest.h @@ -82,7 +82,7 @@ private: void send_create_object_map(); void handle_create_object_map(int r); - Context *start_lock_op(); + Context *start_lock_op(int* r); void finish(int r); }; diff --git a/src/librbd/image/RemoveRequest.cc b/src/librbd/image/RemoveRequest.cc index 36bd9e083581..f28c00bfd28f 100644 --- a/src/librbd/image/RemoveRequest.cc +++ b/src/librbd/image/RemoveRequest.cc @@ -109,7 +109,8 @@ void RemoveRequest::handle_open_image(int r) { template void RemoveRequest::check_exclusive_lock() { if (m_image_ctx->operations_disabled) { - lderr(m_cct) << "image operations disabled due to unsupported op features" << dendl; + lderr(m_cct) << "image operations disabled due to unsupported op features" + << dendl; send_close_image(-EROFS); return; } diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc index 2ef36f582235..2e08b5e92310 100644 --- a/src/librbd/internal.cc +++ b/src/librbd/internal.cc @@ -1155,11 +1155,12 @@ bool compare_by_name(const child_info_t& c1, const child_info_t& c2) int is_exclusive_lock_owner(ImageCtx *ictx, bool *is_owner) { + CephContext *cct = ictx->cct; + ldout(cct, 20) << __func__ << ": ictx=" << ictx << dendl; *is_owner = false; RWLock::RLocker owner_locker(ictx->owner_lock); - if (ictx->exclusive_lock == nullptr || - !ictx->exclusive_lock->is_lock_owner()) { + if (ictx->exclusive_lock == nullptr) { return 0; } @@ -1215,11 +1216,11 @@ bool compare_by_name(const child_info_t& c1, const child_info_t& c2) } RWLock::RLocker l(ictx->owner_lock); - - if (ictx->exclusive_lock == nullptr || - !ictx->exclusive_lock->is_lock_owner()) { + if (ictx->exclusive_lock == nullptr) { + return -EINVAL; + } else if (!ictx->exclusive_lock->is_lock_owner()) { lderr(cct) << "failed to acquire exclusive lock" << dendl; - return -EROFS; + return ictx->exclusive_lock->get_unlocked_op_error(); } return 0; diff --git a/src/librbd/io/ImageRequestWQ.cc b/src/librbd/io/ImageRequestWQ.cc index e5042b5edb32..5d3ed25a91fd 100644 --- a/src/librbd/io/ImageRequestWQ.cc +++ b/src/librbd/io/ImageRequestWQ.cc @@ -7,6 +7,7 @@ #include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/ImageState.h" +#include "librbd/ImageWatcher.h" #include "librbd/internal.h" #include "librbd/Utils.h" #include "librbd/exclusive_lock/Policy.h" @@ -647,7 +648,8 @@ void *ImageRequestWQ::_void_dequeue() { ldout(cct, 5) << "exclusive lock required: delaying IO " << item << dendl; if (!m_image_ctx.get_exclusive_lock_policy()->may_auto_request_lock()) { lderr(cct) << "op requires exclusive lock" << dendl; - fail_in_flight_io(-EROFS, item); + fail_in_flight_io(m_image_ctx.exclusive_lock->get_unlocked_op_error(), + item); // wake up the IO since we won't be returning a request to process this->signal(); diff --git a/src/librbd/mirror/DemoteRequest.cc b/src/librbd/mirror/DemoteRequest.cc index 1fb23a5ddf22..c5d38752ea92 100644 --- a/src/librbd/mirror/DemoteRequest.cc +++ b/src/librbd/mirror/DemoteRequest.cc @@ -105,11 +105,12 @@ void DemoteRequest::handle_acquire_lock(int r) { } m_image_ctx.owner_lock.get_read(); - if (m_image_ctx.exclusive_lock == nullptr || + if (m_image_ctx.exclusive_lock != nullptr && !m_image_ctx.exclusive_lock->is_lock_owner()) { + r = m_image_ctx.exclusive_lock->get_unlocked_op_error(); m_image_ctx.owner_lock.put_read(); lderr(cct) << "failed to acquire exclusive lock" << dendl; - finish(-EROFS); + finish(r); return; } m_image_ctx.owner_lock.put_read(); diff --git a/src/librbd/operation/DisableFeaturesRequest.cc b/src/librbd/operation/DisableFeaturesRequest.cc index a67bc1e18745..5c0335b0b364 100644 --- a/src/librbd/operation/DisableFeaturesRequest.cc +++ b/src/librbd/operation/DisableFeaturesRequest.cc @@ -160,19 +160,20 @@ Context *DisableFeaturesRequest::handle_acquire_exclusive_lock(int *result) { CephContext *cct = image_ctx.cct; ldout(cct, 20) << this << " " << __func__ << ": r=" << *result << dendl; + image_ctx.owner_lock.get_read(); if (*result < 0) { lderr(cct) << "failed to lock image: " << cpp_strerror(*result) << dendl; + image_ctx.owner_lock.put_read(); return handle_finish(*result); - } else if (m_acquired_lock && (image_ctx.exclusive_lock == nullptr || - !image_ctx.exclusive_lock->is_lock_owner())) { + } else if (image_ctx.exclusive_lock != nullptr && + !image_ctx.exclusive_lock->is_lock_owner()) { lderr(cct) << "failed to acquire exclusive lock" << dendl; - *result = -EROFS; + *result = image_ctx.exclusive_lock->get_unlocked_op_error(); + image_ctx.owner_lock.put_read(); return handle_finish(*result); } do { - RWLock::WLocker locker(image_ctx.owner_lock); - m_features &= image_ctx.features; m_new_features = image_ctx.features & ~m_features; m_features_mask = m_features; @@ -202,6 +203,7 @@ Context *DisableFeaturesRequest::handle_acquire_exclusive_lock(int *result) { m_disable_flags |= RBD_FLAG_OBJECT_MAP_INVALID; } } while (false); + image_ctx.owner_lock.put_read(); if (*result < 0) { return handle_finish(*result); diff --git a/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc b/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc index 1120bad1805a..f248e7331c02 100644 --- a/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc +++ b/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc @@ -165,7 +165,7 @@ public: if ((m_src_image_ctx->features & RBD_FEATURE_EXCLUSIVE_LOCK) == 0) { return; } - EXPECT_CALL(mock_exclusive_lock, start_op()).WillOnce( + EXPECT_CALL(mock_exclusive_lock, start_op(_)).WillOnce( ReturnNew([](int) {})); } diff --git a/src/test/librbd/deep_copy/test_mock_SetHeadRequest.cc b/src/test/librbd/deep_copy/test_mock_SetHeadRequest.cc index 9d2d51cdb702..2a30f9a3f444 100644 --- a/src/test/librbd/deep_copy/test_mock_SetHeadRequest.cc +++ b/src/test/librbd/deep_copy/test_mock_SetHeadRequest.cc @@ -58,7 +58,7 @@ public: } void expect_start_op(librbd::MockExclusiveLock &mock_exclusive_lock) { - EXPECT_CALL(mock_exclusive_lock, start_op()).WillOnce( + EXPECT_CALL(mock_exclusive_lock, start_op(_)).WillOnce( ReturnNew([](int) {})); } diff --git a/src/test/librbd/deep_copy/test_mock_SnapshotCopyRequest.cc b/src/test/librbd/deep_copy/test_mock_SnapshotCopyRequest.cc index f774ac0a432b..114a78a4757c 100644 --- a/src/test/librbd/deep_copy/test_mock_SnapshotCopyRequest.cc +++ b/src/test/librbd/deep_copy/test_mock_SnapshotCopyRequest.cc @@ -148,7 +148,7 @@ public: if ((m_src_image_ctx->features & RBD_FEATURE_EXCLUSIVE_LOCK) == 0) { return; } - EXPECT_CALL(mock_exclusive_lock, start_op()).WillOnce( + EXPECT_CALL(mock_exclusive_lock, start_op(_)).WillOnce( ReturnNew([](int) {})); } diff --git a/src/test/librbd/deep_copy/test_mock_SnapshotCreateRequest.cc b/src/test/librbd/deep_copy/test_mock_SnapshotCreateRequest.cc index b43a338baee6..64b7fbd72cf7 100644 --- a/src/test/librbd/deep_copy/test_mock_SnapshotCreateRequest.cc +++ b/src/test/librbd/deep_copy/test_mock_SnapshotCreateRequest.cc @@ -89,7 +89,7 @@ public: } void expect_start_op(librbd::MockExclusiveLock &mock_exclusive_lock) { - EXPECT_CALL(mock_exclusive_lock, start_op()).WillOnce( + EXPECT_CALL(mock_exclusive_lock, start_op(_)).WillOnce( ReturnNew([](int) {})); } diff --git a/src/test/librbd/io/test_mock_ImageRequestWQ.cc b/src/test/librbd/io/test_mock_ImageRequestWQ.cc index 8eb4f0c42ce4..ea23b798ca46 100644 --- a/src/test/librbd/io/test_mock_ImageRequestWQ.cc +++ b/src/test/librbd/io/test_mock_ImageRequestWQ.cc @@ -270,6 +270,49 @@ TEST_F(TestMockIoImageRequestWQ, AcquireLockError) { aio_comp->release(); } +TEST_F(TestMockIoImageRequestWQ, AcquireLockBlacklisted) { + REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockTestImageCtx mock_image_ctx(*ictx); + MockExclusiveLock mock_exclusive_lock; + mock_image_ctx.exclusive_lock = &mock_exclusive_lock; + + auto mock_queued_image_request = new MockImageDispatchSpec(); + expect_was_throttled(*mock_queued_image_request, false); + + InSequence seq; + MockImageRequestWQ mock_image_request_wq(&mock_image_ctx, "io", 60, nullptr); + expect_signal(mock_image_request_wq); + mock_image_request_wq.set_require_lock(DIRECTION_WRITE, true); + + expect_is_write_op(*mock_queued_image_request, true); + expect_queue(mock_image_request_wq); + auto *aio_comp = new librbd::io::AioCompletion(); + mock_image_request_wq.aio_write(aio_comp, 0, 0, {}, 0); + + librbd::exclusive_lock::MockPolicy mock_exclusive_lock_policy; + expect_front(mock_image_request_wq, mock_queued_image_request); + expect_is_refresh_request(mock_image_ctx, false); + expect_is_write_op(*mock_queued_image_request, true); + expect_dequeue(mock_image_request_wq, mock_queued_image_request); + expect_get_exclusive_lock_policy(mock_image_ctx, mock_exclusive_lock_policy); + expect_may_auto_request_lock(mock_exclusive_lock_policy, false); + EXPECT_CALL(*mock_image_ctx.exclusive_lock, get_unlocked_op_error()) + .WillOnce(Return(-EBLACKLISTED)); + expect_process_finish(mock_image_request_wq); + expect_fail(*mock_queued_image_request, -EBLACKLISTED); + expect_is_write_op(*mock_queued_image_request, true); + expect_signal(mock_image_request_wq); + ASSERT_TRUE(mock_image_request_wq.invoke_dequeue() == nullptr); + + ASSERT_EQ(0, aio_comp->wait_for_complete()); + ASSERT_EQ(-EBLACKLISTED, aio_comp->get_return_value()); + aio_comp->release(); +} + TEST_F(TestMockIoImageRequestWQ, RefreshError) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); diff --git a/src/test/librbd/mock/MockExclusiveLock.h b/src/test/librbd/mock/MockExclusiveLock.h index f8eeb0d20a85..1b49fa6a75dc 100644 --- a/src/test/librbd/mock/MockExclusiveLock.h +++ b/src/test/librbd/mock/MockExclusiveLock.h @@ -31,7 +31,7 @@ struct MockExclusiveLock { MOCK_METHOD0(accept_ops, bool()); MOCK_METHOD0(get_unlocked_op_error, int()); - MOCK_METHOD0(start_op, Context*()); + MOCK_METHOD1(start_op, Context*(int*)); }; } // namespace librbd diff --git a/src/test/librbd/test_mock_DeepCopyRequest.cc b/src/test/librbd/test_mock_DeepCopyRequest.cc index 5d394ba73af6..ab2f8acce14c 100644 --- a/src/test/librbd/test_mock_DeepCopyRequest.cc +++ b/src/test/librbd/test_mock_DeepCopyRequest.cc @@ -170,7 +170,7 @@ public: } void expect_start_op(librbd::MockExclusiveLock &mock_exclusive_lock) { - EXPECT_CALL(mock_exclusive_lock, start_op()).WillOnce( + EXPECT_CALL(mock_exclusive_lock, start_op(_)).WillOnce( ReturnNew([](int) {})); } diff --git a/src/test/rbd_mirror/image_deleter/test_mock_SnapshotPurgeRequest.cc b/src/test/rbd_mirror/image_deleter/test_mock_SnapshotPurgeRequest.cc index 370b6d294251..eaa1c8ddb50b 100644 --- a/src/test/rbd_mirror/image_deleter/test_mock_SnapshotPurgeRequest.cc +++ b/src/test/rbd_mirror/image_deleter/test_mock_SnapshotPurgeRequest.cc @@ -139,9 +139,10 @@ public: } void expect_start_op(librbd::MockTestImageCtx &mock_image_ctx, bool success) { - EXPECT_CALL(*mock_image_ctx.exclusive_lock, start_op()) - .WillOnce(Invoke([success]() { + EXPECT_CALL(*mock_image_ctx.exclusive_lock, start_op(_)) + .WillOnce(Invoke([success](int* r) { if (!success) { + *r = -EROFS; return static_cast(nullptr); } return new FunctionContext([](int r) {}); diff --git a/src/tools/rbd_mirror/image_deleter/SnapshotPurgeRequest.cc b/src/tools/rbd_mirror/image_deleter/SnapshotPurgeRequest.cc index d6aebeb56b30..053dcf4656e0 100644 --- a/src/tools/rbd_mirror/image_deleter/SnapshotPurgeRequest.cc +++ b/src/tools/rbd_mirror/image_deleter/SnapshotPurgeRequest.cc @@ -151,10 +151,10 @@ void SnapshotPurgeRequest::snap_unprotect() { << "snap_namespace=" << m_snap_namespace << ", " << "snap_name=" << m_snap_name << dendl; - auto finish_op_ctx = start_lock_op(); + auto finish_op_ctx = start_lock_op(&r); if (finish_op_ctx == nullptr) { derr << "lost exclusive lock" << dendl; - m_ret_val = -EROFS; + m_ret_val = r; close_image(); return; } @@ -205,10 +205,11 @@ void SnapshotPurgeRequest::snap_remove() { << "snap_namespace=" << m_snap_namespace << ", " << "snap_name=" << m_snap_name << dendl; - auto finish_op_ctx = start_lock_op(); + int r; + auto finish_op_ctx = start_lock_op(&r); if (finish_op_ctx == nullptr) { derr << "lost exclusive lock" << dendl; - m_ret_val = -EROFS; + m_ret_val = r; close_image(); return; } @@ -277,9 +278,9 @@ void SnapshotPurgeRequest::finish(int r) { } template -Context *SnapshotPurgeRequest::start_lock_op() { +Context *SnapshotPurgeRequest::start_lock_op(int* r) { RWLock::RLocker owner_locker(m_image_ctx->owner_lock); - return m_image_ctx->exclusive_lock->start_op(); + return m_image_ctx->exclusive_lock->start_op(r); } } // namespace image_deleter diff --git a/src/tools/rbd_mirror/image_deleter/SnapshotPurgeRequest.h b/src/tools/rbd_mirror/image_deleter/SnapshotPurgeRequest.h index 59a13cb8f448..b8b635fe765a 100644 --- a/src/tools/rbd_mirror/image_deleter/SnapshotPurgeRequest.h +++ b/src/tools/rbd_mirror/image_deleter/SnapshotPurgeRequest.h @@ -90,7 +90,7 @@ private: void finish(int r); - Context *start_lock_op(); + Context *start_lock_op(int* r); };