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 <dillaman@redhat.com>
(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 d4a20a717abbe7040ffb819f49aa88cb966200b6..d8bb9ff8a5f773d6f1926a96c2f41271f53fa896 100644 (file)
--- a/src/librbd/DeepCopyRequest.cc
+++ b/src/librbd/DeepCopyRequest.cc
@@ -197,14 +197,15 @@ void DeepCopyRequest<I>::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<I>::handle_copy_object_map(int r) {
 
 template <typename I>
 void DeepCopyRequest<I>::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 b224a7504b43bfb2c42b3831e8668b7011d12c75..a70e77e917c053594b6038c6017a342282ece78c 100644 (file)
--- a/src/librbd/ExclusiveLock.cc
+++ b/src/librbd/ExclusiveLock.cc
@@ -137,11 +137,12 @@ void ExclusiveLock<I>::handle_peer_notification(int r) {
 }
 
 template <typename I>
-Context *ExclusiveLock<I>::start_op() {
+Context *ExclusiveLock<I>::start_op(int* ret_val) {
   assert(m_image_ctx.owner_lock.is_locked());
   Mutex::Locker locker(ML<I>::m_lock);
 
   if (!accept_ops(ML<I>::m_lock)) {
+    *ret_val = get_unlocked_op_error();
     return nullptr;
   }
 

diff --git a/src/librbd/ExclusiveLock.h b/src/librbd/ExclusiveLock.h
index 3c5ebab33a38dbe3d3d71a71b0967bd115ebe9b0..8b2a411f44232debfed2996337bf48b0ec1789f5 100644 (file)
--- 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 67248e2c972f5d55a075c3c28732cd588cd33f54..8c467b61f093dbfbcc88c9ff4aca2e763b0bad3d 100644 (file)
--- 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<I>::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<I>::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<I>::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<I>::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 19072aee7eb1c34aa5a2425cfbf8adfc1e4a42de..9111be5cd895736c01c1c514159d11d0af61dbce 100644 (file)
--- a/src/librbd/deep_copy/ObjectCopyRequest.cc
+++ b/src/librbd/deep_copy/ObjectCopyRequest.cc
@@ -371,14 +371,15 @@ void ObjectCopyRequest<I>::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<I>::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<I>::send_update_object_map() {
   ldout(m_cct, 20) << "dst_snap_id=" << dst_snap_id << ", object_state="
                    << static_cast<uint32_t>(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<I>::handle_update_object_map(int r) {
 }
 
 template <typename I>
-Context *ObjectCopyRequest<I>::start_lock_op(RWLock &owner_lock) {
+Context *ObjectCopyRequest<I>::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 <typename I>

diff --git a/src/librbd/deep_copy/ObjectCopyRequest.h b/src/librbd/deep_copy/ObjectCopyRequest.h
index e514d539876af4a0048af54bdad88956387e6143..2682f92b61313eec65eaa2e88a304d8fec574835 100644 (file)
--- 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 ade87ba384d8b4bc0d3096f36a86430518bf62a5..d82f446509bba2bc3fecc17cc9b0aa1f4388cb82 100644 (file)
--- a/src/librbd/deep_copy/SetHeadRequest.cc
+++ b/src/librbd/deep_copy/SetHeadRequest.cc
@@ -55,10 +55,11 @@ void SetHeadRequest<I>::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<I>::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<I>::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<I>::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<I>::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<I>::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<I>::handle_set_parent(int r) {
 }
 
 template <typename I>
-Context *SetHeadRequest<I>::start_lock_op() {
+Context *SetHeadRequest<I>::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 <typename I>

diff --git a/src/librbd/deep_copy/SetHeadRequest.h b/src/librbd/deep_copy/SetHeadRequest.h
index 7754ff73553514bacad2b1a3ebfa42dc01dcf537..e3c313f1b1c6066857261a3a5aa38bd24783c32f 100644 (file)
--- 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 bc32e7e67a8181a21907dd1641b12a6308a76dfe..597c99bbd390382f0682a6b8e1b4886994283d1b 100644 (file)
--- a/src/librbd/deep_copy/SnapshotCopyRequest.cc
+++ b/src/librbd/deep_copy/SnapshotCopyRequest.cc
@@ -174,10 +174,11 @@ void SnapshotCopyRequest<I>::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<I>::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<I>::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<I>::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<I>::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<I>::send_resize_object_map() {
       }
 
       lderr(m_cct) << "lost exclusive lock" << dendl;
-      r = -EROFS;
     }
   }
 
@@ -644,18 +647,18 @@ int SnapshotCopyRequest<I>::validate_parent(I *image_ctx,
 }
 
 template <typename I>
-Context *SnapshotCopyRequest<I>::start_lock_op() {
+Context *SnapshotCopyRequest<I>::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 <typename I>
-Context *SnapshotCopyRequest<I>::start_lock_op(RWLock &owner_lock) {
+Context *SnapshotCopyRequest<I>::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 <typename I>

diff --git a/src/librbd/deep_copy/SnapshotCopyRequest.h b/src/librbd/deep_copy/SnapshotCopyRequest.h
index 0155488b6dfcb0d6e7839420ee5d04ce46a7b46c..88cf0e52c13abb1966a4d85c8cd9f6b8d8f263e0 100644 (file)
--- 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 a9503765ac29b6db153ac4eaf4096b73dc2f684c..f199f543e38316367996b68b8e8ef0f268fd5a69 100644 (file)
--- a/src/librbd/deep_copy/SnapshotCreateRequest.cc
+++ b/src/librbd/deep_copy/SnapshotCreateRequest.cc
@@ -68,10 +68,11 @@ template <typename I>
 void SnapshotCreateRequest<I>::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<I>::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<I>::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<I>::handle_create_object_map(int r) {
 }
 
 template <typename I>
-Context *SnapshotCreateRequest<I>::start_lock_op() {
+Context *SnapshotCreateRequest<I>::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 <typename I>

diff --git a/src/librbd/deep_copy/SnapshotCreateRequest.h b/src/librbd/deep_copy/SnapshotCreateRequest.h
index 4ddcefd19d969edb3b01e834788c13f3b998fdc0..a34b234bb65530dde1ac2829f1e644c30ecdac2b 100644 (file)
--- 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 36bd9e08358123f940aafe3d1391df2bdb2012a5..f28c00bfd28f8ba07677114eea5a5919e213b8a6 100644 (file)
--- a/src/librbd/image/RemoveRequest.cc
+++ b/src/librbd/image/RemoveRequest.cc
@@ -109,7 +109,8 @@ void RemoveRequest<I>::handle_open_image(int r) {
 template<typename I>
 void RemoveRequest<I>::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 2ef36f582235fb91cbd89838a4ac3f4b5b0bed37..2e08b5e92310685d6ed80cfa0ac9ab4016d5c75f 100644 (file)
--- 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 e5042b5edb32468fd947a69cabade4bada36de11..5d3ed25a91fd8d5bcecde1d69183d8ba27bdd385 100644 (file)
--- 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<I>::_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 1fb23a5ddf2253f5533d3a9ccdf0b46b3e5906cb..c5d38752ea9200b4c1f73a0f7d96210eb7e69c38 100644 (file)
--- a/src/librbd/mirror/DemoteRequest.cc
+++ b/src/librbd/mirror/DemoteRequest.cc
@@ -105,11 +105,12 @@ void DemoteRequest<I>::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 a67bc1e187457ed733bbebe09d125510760b4c9f..5c0335b0b3645241f3c4b7de387fd99df34a2b7e 100644 (file)
--- a/src/librbd/operation/DisableFeaturesRequest.cc
+++ b/src/librbd/operation/DisableFeaturesRequest.cc
@@ -160,19 +160,20 @@ Context *DisableFeaturesRequest<I>::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<I>::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 1120bad1805aa50d09dfa0ab402a8f1cf76f22e1..f248e7331c02a38599cd344ef48e5952779431a3 100644 (file)
--- 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<FunctionContext>([](int) {}));
   }
 

diff --git a/src/test/librbd/deep_copy/test_mock_SetHeadRequest.cc b/src/test/librbd/deep_copy/test_mock_SetHeadRequest.cc
index 9d2d51cdb702b2ad8a604845aa3fb361e225eb74..2a30f9a3f444a85d1f0bb2807b66ac5af95a6c21 100644 (file)
--- 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<FunctionContext>([](int) {}));
   }
 

diff --git a/src/test/librbd/deep_copy/test_mock_SnapshotCopyRequest.cc b/src/test/librbd/deep_copy/test_mock_SnapshotCopyRequest.cc
index f774ac0a432bb824918e1cdf31f48502df5c2485..114a78a4757cb38e056a82386be1de21e4a2a49e 100644 (file)
--- 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<FunctionContext>([](int) {}));
   }
 

diff --git a/src/test/librbd/deep_copy/test_mock_SnapshotCreateRequest.cc b/src/test/librbd/deep_copy/test_mock_SnapshotCreateRequest.cc
index b43a338baee6dcb079243b554bef92a6b17a8f22..64b7fbd72cf7ee27159c4b75ff7379717666f6ff 100644 (file)
--- 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<FunctionContext>([](int) {}));
   }
 

diff --git a/src/test/librbd/io/test_mock_ImageRequestWQ.cc b/src/test/librbd/io/test_mock_ImageRequestWQ.cc
index 8eb4f0c42ce49c6042cda4bc17b4ea85e6c19ed9..ea23b798ca46ef09a8fa1f69e97e2d704b185841 100644 (file)
--- 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 f8eeb0d20a850e4aaece10dee73b13f106f042b7..1b49fa6a75dc18cb983d48fdeeb63fbfbb9e1d77 100644 (file)
--- 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 5d394ba73af65833e4ef2b832f1141ab3b116400..ab2f8acce14cf1c56455c7aa9e58e788d213929f 100644 (file)
--- 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<FunctionContext>([](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 370b6d2942519fa651904d829e9f56a50de03ae4..eaa1c8ddb50badcc74c7597150a8b61b138bb5e0 100644 (file)
--- 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<FunctionContext*>(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 d6aebeb56b30c8dec2c4783ec57d4f790eabed26..053dcf4656e04a341fea399a08dc791375458773 100644 (file)
--- a/src/tools/rbd_mirror/image_deleter/SnapshotPurgeRequest.cc
+++ b/src/tools/rbd_mirror/image_deleter/SnapshotPurgeRequest.cc
@@ -151,10 +151,10 @@ void SnapshotPurgeRequest<I>::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<I>::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<I>::finish(int r) {
 }
 
 template <typename I>
-Context *SnapshotPurgeRequest<I>::start_lock_op() {
+Context *SnapshotPurgeRequest<I>::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 59a13cb8f448252c5deaf468efed923e33a1f08f..b8b635fe765ae5f92f6edc198f26839baf8fc0b9 100644 (file)
--- 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);
 
 };