Merge branch 'wip-15669-jewel' of https://github.com/Abhishekvrshny/ceph

 into wip-15698

src/librbd/AioImageRequestWQ.cc

@@ -231,10 +231,10 @@ bool AioImageRequestWQ::is_lock_request_needed() const {
           (m_require_lock_on_read && m_queued_reads.read() > 0));
 }
 
-void AioImageRequestWQ::block_writes() {
+int AioImageRequestWQ::block_writes() {
   C_SaferCond cond_ctx;
   block_writes(&cond_ctx);
-  cond_ctx.wait();
+  return cond_ctx.wait();
 }
 
 void AioImageRequestWQ::block_writes(Context *on_blocked) {

src/librbd/AioImageRequestWQ.h

@@ -46,7 +46,7 @@ class AioImageRequestWQ : protected ThreadPool::PointerWQ<AioImageRequest<ImageC
     return (m_write_blockers > 0);
   }
 
-  void block_writes();
+  int block_writes();
   void block_writes(Context *on_blocked);
   void unblock_writes();
 

src/librbd/ExclusiveLock.cc

@@ -78,12 +78,32 @@ template <typename I>
 bool ExclusiveLock<I>::accept_requests() const {
   Mutex::Locker locker(m_lock);
 
-  bool accept_requests = (!is_shutdown() && m_state == STATE_LOCKED);
+  bool accept_requests = (!is_shutdown() && m_state == STATE_LOCKED &&
+                          m_request_blockers == 0);
   ldout(m_image_ctx.cct, 20) << this << " " << __func__ << "="
                              << accept_requests << dendl;
   return accept_requests;
 }
 
+template <typename I>
+void ExclusiveLock<I>::block_requests() {
+  Mutex::Locker locker(m_lock);
+  ++m_request_blockers;
+
+  ldout(m_image_ctx.cct, 20) << this << " " << __func__ << "="
+                             << m_request_blockers << dendl;
+}
+
+template <typename I>
+void ExclusiveLock<I>::unblock_requests() {
+  Mutex::Locker locker(m_lock);
+  assert(m_request_blockers > 0);
+  --m_request_blockers;
+
+  ldout(m_image_ctx.cct, 20) << this << " " << __func__ << "="
+                             << m_request_blockers << dendl;
+}
+
 template <typename I>
 void ExclusiveLock<I>::init(uint64_t features, Context *on_init) {
   assert(m_image_ctx.owner_lock.is_locked());

src/librbd/ExclusiveLock.h

@@ -32,6 +32,9 @@ class ExclusiveLock {
   bool is_lock_owner() const;
   bool accept_requests() const;
 
+  void block_requests();
+  void unblock_requests();
+
   void init(uint64_t features, Context *on_init);
   void shut_down(Context *on_shutdown);
 
@@ -127,6 +130,8 @@ class ExclusiveLock {
 
   ActionsContexts m_actions_contexts;
 
+  uint32_t m_request_blockers = 0;
+
   std::string encode_lock_cookie() const;
 
   bool is_transition_state() const;

src/librbd/ImageState.cc

@@ -109,7 +109,7 @@ void ImageState<I>::refresh(Context *on_finish) {
   m_lock.Lock();
   if (is_closed()) {
     m_lock.Unlock();
-    on_finish->complete(0);
+    on_finish->complete(-ESHUTDOWN);
     return;
   }
 
@@ -123,9 +123,12 @@ int ImageState<I>::refresh_if_required() {
   C_SaferCond ctx;
   {
     m_lock.Lock();
-    if (m_last_refresh == m_refresh_seq || is_closed()) {
+    if (m_last_refresh == m_refresh_seq) {
       m_lock.Unlock();
       return 0;
+    } else if (is_closed()) {
+      m_lock.Unlock();
+      return -ESHUTDOWN;
     }
 
     Action action(ACTION_TYPE_REFRESH);

src/librbd/Operations.cc

@@ -79,6 +79,9 @@ struct C_InvokeAsyncRequest : public Context {
    *    . . . . . .   |   . . . . . . . . . . . . . . . . . .
    *    .         .   |   .                                 .
    *    .         v   v   v                                 .
+   *    .       REFRESH_IMAGE (skip if not needed)          .
+   *    .             |                                     .
+   *    .             v                                     .
    *    .       ACQUIRE_LOCK (skip if exclusive lock        .
    *    .             |       disabled or has lock)         .
    *    .             |                                     .
@@ -116,6 +119,35 @@ struct C_InvokeAsyncRequest : public Context {
   }
 
   void send() {
+    send_refresh_image();
+  }
+
+  void send_refresh_image() {
+    if (!image_ctx.state->is_refresh_required()) {
+      send_acquire_exclusive_lock();
+      return;
+    }
+
+    CephContext *cct = image_ctx.cct;
+    ldout(cct, 20) << __func__ << dendl;
+
+    Context *ctx = util::create_context_callback<
+      C_InvokeAsyncRequest<I>,
+      &C_InvokeAsyncRequest<I>::handle_refresh_image>(this);
+    image_ctx.state->refresh(ctx);
+  }
+
+  void handle_refresh_image(int r) {
+    CephContext *cct = image_ctx.cct;
+    ldout(cct, 20) << __func__ << ": r=" << r << dendl;
+
+    RWLock::RLocker owner_locker(image_ctx.owner_lock);
+    if (r < 0) {
+      lderr(cct) << "failed to refresh image: " << cpp_strerror(r) << dendl;
+      complete(r);
+      return;
+    }
+
     send_acquire_exclusive_lock();
   }
 
@@ -193,7 +225,7 @@ struct C_InvokeAsyncRequest : public Context {
 
     ldout(cct, 5) << request_type << " timed out notifying lock owner"
                   << dendl;
-    send_acquire_exclusive_lock();
+    send_refresh_image();
   }
 
   void send_local_request() {
@@ -213,7 +245,7 @@ struct C_InvokeAsyncRequest : public Context {
     ldout(cct, 20) << __func__ << ": r=" << r << dendl;
 
     if (r == -ERESTART) {
-      send_acquire_exclusive_lock();
+      send_refresh_image();
       return;
     }
     complete(r);

src/librbd/image_watcher/NotifyLockOwner.cc

@@ -78,7 +78,7 @@ void NotifyLockOwner::handle_notify(int r) {
   }
 
   if (!lock_owner_responded) {
-    lderr(cct) << ": no lock owners detected" << dendl;
+    ldout(cct, 1) << ": no lock owners detected" << dendl;
     finish(-ETIMEDOUT);
     return;
   }

src/librbd/internal.cc

@@ -1589,40 +1589,53 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force) {
       return -EINVAL;
     }
 
-    {
-      RWLock::RLocker owner_locker(ictx->owner_lock);
-      RWLock::WLocker md_locker(ictx->md_lock);
-      r = ictx->flush();
-      if (r < 0) {
-        return r;
-      }
+    RWLock::RLocker owner_locker(ictx->owner_lock);
+    r = ictx->aio_work_queue->block_writes();
+    BOOST_SCOPE_EXIT_ALL( (ictx) ) {
+      ictx->aio_work_queue->unblock_writes();
+    };
+    if (r < 0) {
+      return r;
+    }
+
+    uint64_t disable_mask = (RBD_FEATURES_MUTABLE |
+                             RBD_FEATURES_DISABLE_ONLY);
+    if ((enabled && (features & RBD_FEATURES_MUTABLE) != features) ||
+        (!enabled && (features & disable_mask) != features)) {
+      lderr(cct) << "cannot update immutable features" << dendl;
+      return -EINVAL;
+    } else if (features == 0) {
+      lderr(cct) << "update requires at least one feature" << dendl;
+      return -EINVAL;
+    }
 
-      uint64_t disable_mask = (RBD_FEATURES_MUTABLE |
-                               RBD_FEATURES_DISABLE_ONLY);
-      if ((enabled && (features & RBD_FEATURES_MUTABLE) != features) ||
-          (!enabled && (features & disable_mask) != features)) {
-        lderr(cct) << "cannot update immutable features" << dendl;
-        return -EINVAL;
-      } else if (features == 0) {
-        lderr(cct) << "update requires at least one feature" << dendl;
-        return -EINVAL;
+    // avoid accepting new requests from peers while we manipulate
+    // the image features
+    if (ictx->exclusive_lock != nullptr) {
+      ictx->exclusive_lock->block_requests();
+    }
+    BOOST_SCOPE_EXIT_ALL( (ictx) ) {
+      if (ictx->exclusive_lock != nullptr) {
+        ictx->exclusive_lock->unblock_requests();
       }
+    };
 
-      // if disabling features w/ exclusive lock supported, we need to
-      // acquire the lock to temporarily block IO against the image
-      if (ictx->exclusive_lock != nullptr && !enabled) {
-        C_SaferCond lock_ctx;
-        ictx->exclusive_lock->request_lock(&lock_ctx);
-        r = lock_ctx.wait();
-        if (r < 0) {
-          lderr(cct) << "failed to lock image: " << cpp_strerror(r) << dendl;
-          return r;
-        } else if (!ictx->exclusive_lock->is_lock_owner()) {
-          lderr(cct) << "failed to acquire exclusive lock" << dendl;
-          return -EROFS;
-        }
+    // if disabling features w/ exclusive lock supported, we need to
+    // acquire the lock to temporarily block IO against the image
+    if (ictx->exclusive_lock != nullptr && !enabled) {
+      C_SaferCond lock_ctx;
+      ictx->exclusive_lock->request_lock(&lock_ctx);
+      r = lock_ctx.wait();
+      if (r < 0) {
+        lderr(cct) << "failed to lock image: " << cpp_strerror(r) << dendl;
+        return r;
+      } else if (!ictx->exclusive_lock->is_lock_owner()) {
+        lderr(cct) << "failed to acquire exclusive lock" << dendl;
+        return -EROFS;
       }
+    }
 
+    {
       RWLock::WLocker snap_locker(ictx->snap_lock);
       uint64_t new_features;
       if (enabled) {
@@ -1799,7 +1812,7 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force) {
           img_ctx->state->close();
         }
       }
-   }
+    }
 
     ictx->notify_update();
     return 0;

src/librbd/operation/FlattenRequest.cc

@@ -67,7 +67,10 @@ bool FlattenRequest<I>::should_complete(int r) {
   I &image_ctx = this->m_image_ctx;
   CephContext *cct = image_ctx.cct;
   ldout(cct, 5) << this << " should_complete: " << " r=" << r << dendl;
-  if (r < 0 && !(r == -ENOENT && m_ignore_enoent) ) {
+  if (r == -ERESTART) {
+    ldout(cct, 5) << "flatten operation interrupted" << dendl;
+    return true;
+  } else if (r < 0 && !(r == -ENOENT && m_ignore_enoent) ) {
     lderr(cct) << "flatten encountered an error: " << cpp_strerror(r) << dendl;
     return true;
   }

src/librbd/operation/RebuildObjectMapRequest.cc

@@ -225,7 +225,10 @@ bool RebuildObjectMapRequest<I>::should_complete(int r) {
     break;
   }
 
-  if (r < 0) {
+  if (r == -ERESTART) {
+    ldout(cct, 5) << "rebuild object map operation interrupted" << dendl;
+    return true;
+  } else if (r < 0) {
     lderr(cct) << "rebuild object map encountered an error: " << cpp_strerror(r)
                << dendl;
     return true;

src/librbd/operation/ResizeRequest.cc

@@ -161,7 +161,10 @@ Context *ResizeRequest<I>::handle_trim_image(int *result) {
   CephContext *cct = image_ctx.cct;
   ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl;
 
-  if (*result < 0) {
+  if (*result == -ERESTART) {
+    ldout(cct, 5) << "resize operation interrupted" << dendl;
+    return this->create_context_finisher();
+  } else if (*result < 0) {
     lderr(cct) << "failed to trim image: " << cpp_strerror(*result) << dendl;
     return this->create_context_finisher();
   }

src/librbd/operation/SnapshotRollbackRequest.cc

@@ -222,7 +222,10 @@ Context *SnapshotRollbackRequest<I>::handle_rollback_objects(int *result) {
   CephContext *cct = image_ctx.cct;
   ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl;
 
-  if (*result < 0) {
+  if (*result == -ERESTART) {
+    ldout(cct, 5) << "snapshot rollback operation interrupted" << dendl;
+    return this->create_context_finisher();
+  } else if (*result < 0) {
     lderr(cct) << "failed to rollback objects: " << cpp_strerror(*result)
                << dendl;
     return this->create_context_finisher();

src/librbd/operation/SnapshotUnprotectRequest.cc

@@ -259,7 +259,7 @@ void SnapshotUnprotectRequest<I>::send_scan_pool_children() {
     boost::lambda::bind(boost::lambda::new_ptr<C_ScanPoolChildren<I> >(),
       boost::lambda::_1, &image_ctx, pspec, pools, boost::lambda::_2));
   AsyncObjectThrottle<I> *throttle = new AsyncObjectThrottle<I>(
-    this, image_ctx, context_factory, ctx, NULL, 0, pools.size());
+    nullptr, image_ctx, context_factory, ctx, NULL, 0, pools.size());
   throttle->start_ops(image_ctx.concurrent_management_ops);
 }
 

src/librbd/operation/TrimRequest.cc

@@ -122,7 +122,10 @@ bool TrimRequest<I>::should_complete(int r)
   I &image_ctx = this->m_image_ctx;
   CephContext *cct = image_ctx.cct;
   ldout(cct, 5) << this << " should_complete: r=" << r << dendl;
-  if (r < 0) {
+  if (r == -ERESTART) {
+    ldout(cct, 5) << "trim operation interrupted" << dendl;
+    return true;
+  } else if (r < 0) {
     lderr(cct) << "trim encountered an error: " << cpp_strerror(r) << dendl;
     return true;
   }