librbd: refresh image asynchronously from watch/notify path

The AIO path already ensures asynchronous image refreshes, but the
watch/notify path for maintenance ops was still using the sync refresh
path.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc
index 9347d44b844ac2856d711eaf6434524075de922c..0f0b22434de8d20b53bb99c552d5b9ad42664469 100644 (file)
--- a/src/librbd/ImageWatcher.cc
+++ b/src/librbd/ImageWatcher.cc
@@ -783,6 +783,16 @@ bool ImageWatcher::handle_payload(const UnknownPayload &payload,
   return true;
 }
 
+void ImageWatcher::process_payload(uint64_t notify_id, uint64_t handle,
+                                   const Payload &payload, int r) {
+  if (r < 0) {
+    bufferlist out_bl;
+    acknowledge_notify(notify_id, handle, out_bl);
+  } else {
+    apply_visitor(HandlePayloadVisitor(this, notify_id, handle), payload);
+  }
+}
+
 void ImageWatcher::handle_notify(uint64_t notify_id, uint64_t handle,
                                 bufferlist &bl) {
   NotifyMessage notify_message;
@@ -800,8 +810,13 @@ void ImageWatcher::handle_notify(uint64_t notify_id, uint64_t handle,
     }
   }
 
-  apply_visitor(HandlePayloadVisitor(this, notify_id, handle),
-               notify_message.payload);
+  // if an image refresh is required, refresh before processing the request
+  if (m_image_ctx.state->is_refresh_required()) {
+    m_image_ctx.state->refresh(new C_ProcessPayload(this, notify_id, handle,
+                                                    notify_message.payload));
+  } else {
+    process_payload(notify_id, handle, notify_message.payload, 0);
+  }
 }
 
 void ImageWatcher::handle_error(uint64_t handle, int err) {

diff --git a/src/librbd/ImageWatcher.h b/src/librbd/ImageWatcher.h
index 9784227882433b52afe8fec48c748591606eca76..c82ca55a29612df33d199cfd8a92129c55210812 100644 (file)
--- a/src/librbd/ImageWatcher.h
+++ b/src/librbd/ImageWatcher.h
@@ -169,6 +169,23 @@ private:
     virtual void finish(int r);
   };
 
+  struct C_ProcessPayload : public Context {
+    ImageWatcher *image_watcher;
+    uint64_t notify_id;
+    uint64_t handle;
+    watch_notify::Payload payload;
+
+    C_ProcessPayload(ImageWatcher *image_watcher_, uint64_t notify_id_,
+                     uint64_t handle_, const watch_notify::Payload &payload)
+      : image_watcher(image_watcher_), notify_id(notify_id_), handle(handle_),
+        payload(payload) {
+    }
+
+    virtual void finish(int r) override {
+      image_watcher->process_payload(notify_id, handle, payload, r);
+    }
+  };
+
   struct HandlePayloadVisitor : public boost::static_visitor<void> {
     ImageWatcher *image_watcher;
     uint64_t notify_id;
@@ -265,6 +282,8 @@ private:
                       C_NotifyAck *ctx);
   bool handle_payload(const watch_notify::UnknownPayload& payload,
                       C_NotifyAck *ctx);
+  void process_payload(uint64_t notify_id, uint64_t handle,
+                       const watch_notify::Payload &payload, int r);
 
   void handle_notify(uint64_t notify_id, uint64_t handle, bufferlist &bl);
   void handle_error(uint64_t cookie, int err);

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index acf6d959bc84ca7e1d543406246dcbfa9fbab3b3..b3cae8973b48bb5fa833e5a88dc887650c916698 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -807,12 +807,6 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
     ldout(ictx->cct, 20) << "snap_create_helper " << ictx << " " << snap_name
                          << dendl;
 
-    int r = ictx->state->refresh_if_required(ictx->owner_lock);
-    if (r < 0) {
-      ctx->complete(r);
-      return;
-    }
-
     operation::SnapshotCreateRequest<> *req =
       new operation::SnapshotCreateRequest<>(*ictx, ctx, snap_name);
     req->send();
@@ -878,12 +872,6 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
     ldout(ictx->cct, 20) << "snap_remove_helper " << ictx << " " << snap_name
                          << dendl;
 
-    int r = ictx->state->refresh_if_required(ictx->owner_lock);
-    if (r < 0) {
-      ctx->complete(r);
-      return;
-    }
-
     uint64_t snap_id;
     {
       RWLock::RLocker snap_locker(ictx->snap_lock);
@@ -895,7 +883,7 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
       }
 
       bool is_protected;
-      r = ictx->is_snap_protected(snap_id, &is_protected);
+      int r = ictx->is_snap_protected(snap_id, &is_protected);
       if (r < 0) {
         ctx->complete(r);
         return;
@@ -971,12 +959,6 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
     ldout(ictx->cct, 20) << __func__ << " " << ictx << " from "
                          << src_snap_id << " to " << dst_name << dendl;
 
-    int r = ictx->state->refresh_if_required(ictx->owner_lock);
-    if (r < 0) {
-      ctx->complete(r);
-      return;
-    }
-
     operation::SnapshotRenameRequest<> *req =
       new operation::SnapshotRenameRequest<>(*ictx, ctx, src_snap_id, dst_name);
     req->send();
@@ -1044,12 +1026,6 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
     ldout(ictx->cct, 20) << "snap_protect_helper " << ictx << " " << snap_name
                          << dendl;
 
-    int r = ictx->state->refresh_if_required(ictx->owner_lock);
-    if (r < 0) {
-      ctx->complete(r);
-      return;
-    }
-
     operation::SnapshotProtectRequest<> *request =
       new operation::SnapshotProtectRequest<>(*ictx, ctx, snap_name);
     request->send();
@@ -1119,12 +1095,6 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
     ldout(ictx->cct, 20) << "snap_unprotect_helper " << ictx << " " << snap_name
                          << dendl;
 
-    int r = ictx->state->refresh_if_required(ictx->owner_lock);
-    if (r < 0) {
-      ctx->complete(r);
-      return;
-    }
-
     operation::SnapshotUnprotectRequest<> *request =
       new operation::SnapshotUnprotectRequest<>(*ictx, ctx, snap_name);
     request->send();
@@ -1737,12 +1707,6 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
     ldout(ictx->cct, 20) << "rename_helper " << ictx << " " << dstname
                          << dendl;
 
-    int r = ictx->state->refresh_if_required(ictx->owner_lock);
-    if (r < 0) {
-      ctx->complete(r);
-      return;
-    }
-
     operation::RenameRequest<> *req =
       new operation::RenameRequest<>(*ictx, ctx, dstname);
     req->send();
@@ -1804,136 +1768,138 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
       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);
+      RWLock::WLocker md_locker(ictx->md_lock);
+      r = ictx->flush();
+      if (r < 0) {
+        return r;
+      }
 
-    if ((features & RBD_FEATURES_MUTABLE) != 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;
-    }
+      if ((features & RBD_FEATURES_MUTABLE) != 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;
+      }
 
-    RWLock::WLocker snap_locker(ictx->snap_lock);
-    uint64_t new_features;
-    if (enabled) {
-      features &= ~ictx->features;
-      new_features = ictx->features | features;
-    } else {
-      features &= ictx->features;
-      new_features = ictx->features & ~features;
-    }
+      RWLock::WLocker snap_locker(ictx->snap_lock);
+      uint64_t new_features;
+      if (enabled) {
+        features &= ~ictx->features;
+        new_features = ictx->features | features;
+      } else {
+        features &= ictx->features;
+        new_features = ictx->features & ~features;
+      }
 
-    if (features == 0) {
-      return 0;
-    }
+      if (features == 0) {
+        return 0;
+      }
 
-    uint64_t features_mask = features;
-    uint64_t disable_flags = 0;
-    if (enabled) {
-      uint64_t enable_flags = 0;
+      uint64_t features_mask = features;
+      uint64_t disable_flags = 0;
+      if (enabled) {
+        uint64_t enable_flags = 0;
 
-      if ((features & RBD_FEATURE_OBJECT_MAP) != 0) {
-        if ((new_features & RBD_FEATURE_EXCLUSIVE_LOCK) == 0) {
-          lderr(cct) << "cannot enable object map" << dendl;
-          return -EINVAL;
+        if ((features & RBD_FEATURE_OBJECT_MAP) != 0) {
+          if ((new_features & RBD_FEATURE_EXCLUSIVE_LOCK) == 0) {
+            lderr(cct) << "cannot enable object map" << dendl;
+            return -EINVAL;
+          }
+          enable_flags |= RBD_FLAG_OBJECT_MAP_INVALID;
+          features_mask |= RBD_FEATURE_EXCLUSIVE_LOCK;
         }
-        enable_flags |= RBD_FLAG_OBJECT_MAP_INVALID;
-        features_mask |= RBD_FEATURE_EXCLUSIVE_LOCK;
-      }
-      if ((features & RBD_FEATURE_FAST_DIFF) != 0) {
-        if ((new_features & RBD_FEATURE_OBJECT_MAP) == 0) {
-          lderr(cct) << "cannot enable fast diff" << dendl;
-          return -EINVAL;
+        if ((features & RBD_FEATURE_FAST_DIFF) != 0) {
+          if ((new_features & RBD_FEATURE_OBJECT_MAP) == 0) {
+            lderr(cct) << "cannot enable fast diff" << dendl;
+            return -EINVAL;
+          }
+          enable_flags |= RBD_FLAG_FAST_DIFF_INVALID;
+          features_mask |= (RBD_FEATURE_OBJECT_MAP | RBD_FEATURE_EXCLUSIVE_LOCK);
         }
-        enable_flags |= RBD_FLAG_FAST_DIFF_INVALID;
-        features_mask |= (RBD_FEATURE_OBJECT_MAP | RBD_FEATURE_EXCLUSIVE_LOCK);
-      }
-      if ((features & RBD_FEATURE_JOURNALING) != 0) {
-        if ((new_features & RBD_FEATURE_EXCLUSIVE_LOCK) == 0) {
-          lderr(cct) << "cannot enable journaling" << dendl;
-          return -EINVAL;
+        if ((features & RBD_FEATURE_JOURNALING) != 0) {
+          if ((new_features & RBD_FEATURE_EXCLUSIVE_LOCK) == 0) {
+            lderr(cct) << "cannot enable journaling" << dendl;
+            return -EINVAL;
+          }
+          features_mask |= RBD_FEATURE_EXCLUSIVE_LOCK;
+
+          r = Journal::create(ictx->md_ctx, ictx->id, ictx->journal_order,
+                           ictx->journal_splay_width,
+                           ictx->journal_pool);
+          if (r < 0) {
+            lderr(cct) << "error creating image journal: " << cpp_strerror(r)
+                       << dendl;
+            return r;
+          }
         }
-        features_mask |= RBD_FEATURE_EXCLUSIVE_LOCK;
 
-        r = Journal::create(ictx->md_ctx, ictx->id, ictx->journal_order,
-                           ictx->journal_splay_width,
-                           ictx->journal_pool);
-        if (r < 0) {
-          lderr(cct) << "error creating image journal: " << cpp_strerror(r)
-                     << dendl;
-          return r;
+        if (enable_flags != 0) {
+          r = update_all_flags(ictx, enable_flags, enable_flags);
+          if (r < 0) {
+            return r;
+          }
         }
-      }
-
-      if (enable_flags != 0) {
-        r = update_all_flags(ictx, enable_flags, enable_flags);
-        if (r < 0) {
-          return r;
+      } else {
+        if ((features & RBD_FEATURE_EXCLUSIVE_LOCK) != 0) {
+          if ((new_features & RBD_FEATURE_OBJECT_MAP) != 0 ||
+              (new_features & RBD_FEATURE_JOURNALING) != 0) {
+            lderr(cct) << "cannot disable exclusive lock" << dendl;
+            return -EINVAL;
+          }
+          features_mask |= RBD_FEATURE_OBJECT_MAP;
         }
-      }
-    } else {
-      if ((features & RBD_FEATURE_EXCLUSIVE_LOCK) != 0) {
-        if ((new_features & RBD_FEATURE_OBJECT_MAP) != 0 ||
-            (new_features & RBD_FEATURE_JOURNALING) != 0) {
-          lderr(cct) << "cannot disable exclusive lock" << dendl;
-          return -EINVAL;
+        if ((features & RBD_FEATURE_OBJECT_MAP) != 0) {
+          if ((new_features & RBD_FEATURE_FAST_DIFF) != 0) {
+            lderr(cct) << "cannot disable object map" << dendl;
+            return -EINVAL;
+          }
+
+          disable_flags = RBD_FLAG_OBJECT_MAP_INVALID;
+          r = remove_object_map(ictx);
+          if (r < 0) {
+            lderr(cct) << "failed to remove object map" << dendl;
+            return r;
+          }
         }
-        features_mask |= RBD_FEATURE_OBJECT_MAP;
-      }
-      if ((features & RBD_FEATURE_OBJECT_MAP) != 0) {
-        if ((new_features & RBD_FEATURE_FAST_DIFF) != 0) {
-          lderr(cct) << "cannot disable object map" << dendl;
-          return -EINVAL;
+        if ((features & RBD_FEATURE_FAST_DIFF) != 0) {
+          disable_flags = RBD_FLAG_FAST_DIFF_INVALID;
         }
-
-        disable_flags = RBD_FLAG_OBJECT_MAP_INVALID;
-        r = remove_object_map(ictx);
-        if (r < 0) {
-          lderr(cct) << "failed to remove object map" << dendl;
-          return r;
+        if ((features & RBD_FEATURE_JOURNALING) != 0) {
+          r = Journal::remove(ictx->md_ctx, ictx->id);
+          if (r < 0) {
+            lderr(cct) << "error removing image journal: " << cpp_strerror(r)
+                       << dendl;
+            return r;
+          }
         }
       }
-      if ((features & RBD_FEATURE_FAST_DIFF) != 0) {
-        disable_flags = RBD_FLAG_FAST_DIFF_INVALID;
+
+      ldout(cct, 10) << "update_features: features=" << new_features << ", "
+                     << "mask=" << features_mask << dendl;
+      r = librbd::cls_client::set_features(&ictx->md_ctx, ictx->header_oid,
+                                           new_features, features_mask);
+      if (r < 0) {
+        lderr(cct) << "failed to update features: " << cpp_strerror(r)
+                   << dendl;
+        return r;
       }
-      if ((features & RBD_FEATURE_JOURNALING) != 0) {
-        r = Journal::remove(ictx->md_ctx, ictx->id);
+      if (((ictx->features & RBD_FEATURE_OBJECT_MAP) == 0) &&
+        ((features & RBD_FEATURE_OBJECT_MAP) != 0)) {
+        r = create_object_map(ictx);
         if (r < 0) {
-          lderr(cct) << "error removing image journal: " << cpp_strerror(r)
-                     << dendl;
+          lderr(cct) << "failed to create object map" << dendl;
           return r;
         }
       }
-    }
 
-    ldout(cct, 10) << "update_features: features=" << new_features << ", mask="
-                   << features_mask << dendl;
-    r = librbd::cls_client::set_features(&ictx->md_ctx, ictx->header_oid,
-                                         new_features, features_mask);
-    if (r < 0) {
-      lderr(cct) << "failed to update features: " << cpp_strerror(r)
-                 << dendl;
-      return r;
-    }
-    if (((ictx->features & RBD_FEATURE_OBJECT_MAP) == 0) &&
-      ((features & RBD_FEATURE_OBJECT_MAP) != 0)) {
-      r = create_object_map(ictx);
-      if (r < 0) {
-        lderr(cct) << "failed to create object map" << dendl;
-        return r;
-      }
-    }
-
-    if (disable_flags != 0) {
-      r = update_all_flags(ictx, 0, disable_flags);
-      if (r < 0) {
-        return r;
+      if (disable_flags != 0) {
+        r = update_all_flags(ictx, 0, disable_flags);
+        if (r < 0) {
+          return r;
+        }
       }
     }
 
@@ -2216,12 +2182,6 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
                   << size << dendl;
     ictx->snap_lock.put_read();
 
-    int r = ictx->state->refresh_if_required(ictx->owner_lock);
-    if (r < 0) {
-      ctx->complete(r);
-      return;
-    }
-
     {
       RWLock::RLocker snap_locker(ictx->snap_lock);
       if (ictx->snap_id != CEPH_NOSNAP || ictx->read_only) {
@@ -2595,13 +2555,6 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
     CephContext *cct = ictx->cct;
     ldout(cct, 20) << "flatten" << dendl;
 
-    int r;
-    if ((r = ictx->state->refresh_if_required(ictx->owner_lock)) < 0) {
-      lderr(cct) << "ictx_check failed" << dendl;
-      ctx->complete(r);
-      return;
-    }
-
     uint64_t object_size;
     uint64_t overlap_objects;
     ::SnapContext snapc;
@@ -2630,7 +2583,7 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
 
       snapc = ictx->snapc;
       assert(ictx->parent != NULL);
-      r = ictx->get_parent_overlap(CEPH_NOSNAP, &overlap);
+      int r = ictx->get_parent_overlap(CEPH_NOSNAP, &overlap);
       assert(r == 0);
       assert(overlap <= ictx->size);
 
@@ -2685,12 +2638,6 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
       return;
     }
 
-    int r = ictx->state->refresh_if_required(ictx->owner_lock);
-    if (r < 0) {
-      ctx->complete(r);
-      return;
-    }
-
     operation::RebuildObjectMapRequest<> *req =
       new operation::RebuildObjectMapRequest<>(*ictx, ctx, prog_ctx);
     req->send();
@@ -2744,12 +2691,16 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
      * checks that we think we will succeed. But for now, let's not
      * duplicate that code.
      */
-    RWLock::RLocker locker(ictx->md_lock);
-    r = rados::cls::lock::lock(&ictx->md_ctx, ictx->header_oid, RBD_LOCK_NAME,
-                              exclusive ? LOCK_EXCLUSIVE : LOCK_SHARED,
-                              cookie, tag, "", utime_t(), 0);
-    if (r < 0)
-      return r;
+    {
+      RWLock::RLocker locker(ictx->md_lock);
+      r = rados::cls::lock::lock(&ictx->md_ctx, ictx->header_oid, RBD_LOCK_NAME,
+                                exclusive ? LOCK_EXCLUSIVE : LOCK_SHARED,
+                                cookie, tag, "", utime_t(), 0);
+      if (r < 0) {
+        return r;
+      }
+    }
+
     notify_change(ictx->md_ctx, ictx->header_oid, ictx);
     return 0;
   }
@@ -2763,11 +2714,15 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
     if (r < 0)
       return r;
 
-    RWLock::RLocker locker(ictx->md_lock);
-    r = rados::cls::lock::unlock(&ictx->md_ctx, ictx->header_oid,
-                                RBD_LOCK_NAME, cookie);
-    if (r < 0)
-      return r;
+    {
+      RWLock::RLocker locker(ictx->md_lock);
+      r = rados::cls::lock::unlock(&ictx->md_ctx, ictx->header_oid,
+                                  RBD_LOCK_NAME, cookie);
+      if (r < 0) {
+        return r;
+      }
+    }
+
     notify_change(ictx->md_ctx, ictx->header_oid, ictx);
     return 0;
   }
@@ -2815,7 +2770,7 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
       if (client_address.empty()) {
         return -ENOENT;
       }
-      
+
       RWLock::RLocker locker(ictx->md_lock);
       librados::Rados rados(ictx->md_ctx);
       r = rados.blacklist_add(client_address,