librbd: async ops should return status via the Context

Avoid handling a return value which could indicate an error and
instead return initialization errors via the Context callback.

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

diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc
index 66382dccd44e4dec0b2dd704374a07df2d460ddc..87aebf0d9084fd4658390c628af01e62b94484f8 100644 (file)
--- a/src/librbd/ImageWatcher.cc
+++ b/src/librbd/ImageWatcher.cc
@@ -872,14 +872,6 @@ int ImageWatcher::prepare_async_request(const AsyncRequestId& async_request_id,
   return 0;
 }
 
-void ImageWatcher::cleanup_async_request(const AsyncRequestId& async_request_id,
-                                         Context *ctx) {
-  delete ctx;
-
-  RWLock::WLocker l(m_async_request_lock);
-  m_async_pending.erase(async_request_id);
-}
-
 void ImageWatcher::handle_payload(const HeaderUpdatePayload &payload,
                                  bufferlist *out) {
   ldout(m_image_ctx.cct, 10) << this << " image header updated" << dendl;
@@ -1020,12 +1012,7 @@ void ImageWatcher::handle_payload(const FlattenPayload &payload,
     if (new_request) {
       ldout(m_image_ctx.cct, 10) << this << " remote flatten request: "
                                 << payload.async_request_id << dendl;
-      r = librbd::async_flatten(&m_image_ctx, ctx, *prog_ctx);
-      if (r < 0) {
-       lderr(m_image_ctx.cct) << this << " remove flatten request failed: "
-                              << cpp_strerror(r) << dendl;
-        cleanup_async_request(payload.async_request_id, ctx);
-      }
+      librbd::async_flatten(&m_image_ctx, ctx, *prog_ctx);
     }
 
     ::encode(ResponseMessage(r), *out);
@@ -1045,12 +1032,7 @@ void ImageWatcher::handle_payload(const ResizePayload &payload,
       ldout(m_image_ctx.cct, 10) << this << " remote resize request: "
                                 << payload.async_request_id << " "
                                 << payload.size << dendl;
-      r = librbd::async_resize(&m_image_ctx, ctx, payload.size, *prog_ctx);
-      if (r < 0) {
-       lderr(m_image_ctx.cct) << this << " remove resize request failed: "
-                              << cpp_strerror(r) << dendl;
-        cleanup_async_request(payload.async_request_id, ctx);
-      }
+      librbd::async_resize(&m_image_ctx, ctx, payload.size, *prog_ctx);
     }
 
     ::encode(ResponseMessage(r), *out);
@@ -1064,12 +1046,10 @@ void ImageWatcher::handle_payload(const SnapCreatePayload &payload,
     ldout(m_image_ctx.cct, 10) << this << " remote snap_create request: "
                               << payload.snap_name << dendl;
     C_SaferCond cond_ctx;
-    int r = librbd::snap_create_helper(&m_image_ctx, &cond_ctx,
-                                       payload.snap_name.c_str());
-    if (r == 0) {
-      r = cond_ctx.wait();
-    }
+    librbd::snap_create_helper(&m_image_ctx, &cond_ctx,
+                               payload.snap_name.c_str());
 
+    int r = cond_ctx.wait();
     ::encode(ResponseMessage(r), *out);
   }
 }
@@ -1079,16 +1059,14 @@ void ImageWatcher::handle_payload(const SnapRenamePayload &payload,
   RWLock::RLocker l(m_image_ctx.owner_lock);
   if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) {
     ldout(m_image_ctx.cct, 10) << this << " remote snap_rename request: "
-                              << payload.src_snap_id << " to " 
+                              << payload.snap_id << " to " 
                               << payload.snap_name << dendl;
     C_SaferCond cond_ctx;
-    int r = librbd::snap_rename_helper(&m_image_ctx, &cond_ctx,
-                                       payload.src_snap_id,
-                                       payload.snap_name.c_str());
-    if (r == 0) {
-      r = cond_ctx.wait();
-    }
+    librbd::snap_rename_helper(&m_image_ctx, &cond_ctx,
+                               payload.snap_id,
+                               payload.snap_name.c_str());
 
+    int r = cond_ctx.wait();
     ::encode(ResponseMessage(r), *out);
   }
 }
@@ -1099,12 +1077,10 @@ void ImageWatcher::handle_payload(const SnapRemovePayload &payload,
     ldout(m_image_ctx.cct, 10) << this << " remote snap_remove request: "
                               << payload.snap_name << dendl;
     C_SaferCond cond_ctx;
-    int r = librbd::snap_remove_helper(&m_image_ctx, &cond_ctx,
-                                       payload.snap_name.c_str());
-    if (r == 0) {
-      r = cond_ctx.wait();
-    }
+    librbd::snap_remove_helper(&m_image_ctx, &cond_ctx,
+                               payload.snap_name.c_str());
 
+    int r = cond_ctx.wait();
     ::encode(ResponseMessage(r), *out);
   }
 }
@@ -1146,13 +1122,7 @@ void ImageWatcher::handle_payload(const RebuildObjectMapPayload& payload,
       ldout(m_image_ctx.cct, 10) << this
                                  << " remote rebuild object map request: "
                                  << payload.async_request_id << dendl;
-      r = librbd::async_rebuild_object_map(&m_image_ctx, ctx, *prog_ctx);
-      if (r < 0) {
-        lderr(m_image_ctx.cct) << this
-                               << " remove rebuild object map request failed: "
-                               << cpp_strerror(r) << dendl;
-        cleanup_async_request(payload.async_request_id, ctx);
-      }
+      librbd::async_rebuild_object_map(&m_image_ctx, ctx, *prog_ctx);
     }
 
     ::encode(ResponseMessage(0), *out);

diff --git a/src/librbd/ImageWatcher.h b/src/librbd/ImageWatcher.h
index 6e2a4c5eb3463165d7ed190738edf8e290b293b8..57227653d770e0ddf5aa8381d43db31bdb21912b 100644 (file)
--- a/src/librbd/ImageWatcher.h
+++ b/src/librbd/ImageWatcher.h
@@ -271,8 +271,6 @@ private:
   int prepare_async_request(const watch_notify::AsyncRequestId& id,
                             bool* new_request, Context** ctx,
                             ProgressContext** prog_ctx);
-  void cleanup_async_request(const watch_notify::AsyncRequestId& id,
-                             Context *ctx);
 
   void handle_payload(const watch_notify::HeaderUpdatePayload& payload,
                       bufferlist *out);

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index 59dce0a6e99ebf11c87a2e6530baa861618f0aa8..accf5e0fdfa80ace40f23fd5439cb474b4f8e381 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -177,7 +177,7 @@ int prepare_image_update(ImageCtx *ictx) {
 
 int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
                          bool permit_snapshot,
-                         const boost::function<int(Context*)>& local_request,
+                         const boost::function<void(Context*)>& local_request,
                          const boost::function<int()>& remote_request) {
   int r;
   do {
@@ -208,10 +208,7 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
                             << dendl;
       }
 
-      r = local_request(&ctx);
-      if (r < 0) {
-        return r;
-      }
+      local_request(&ctx);
     }
 
     r = ctx.wait();
@@ -719,7 +716,7 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
     return 0;
   }
 
-  int snap_create_helper(ImageCtx* ictx, Context* ctx, const char* snap_name) {
+  void snap_create_helper(ImageCtx* ictx, Context* ctx, const char* snap_name) {
     assert(ictx->owner_lock.is_locked());
     assert(!ictx->image_watcher->is_lock_supported() ||
           ictx->image_watcher->is_lock_owner());
@@ -729,13 +726,13 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
 
     int r = ictx_check(ictx, ictx->owner_lock);
     if (r < 0) {
-      return r;
+      ctx->complete(r);
+      return;
     }
 
     operation::SnapshotCreateRequest *req =
       new operation::SnapshotCreateRequest(*ictx, ctx, snap_name);
     req->send();
-    return 0;
   }
 
   int snap_remove(ImageCtx *ictx, const char *snap_name)
@@ -770,10 +767,7 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
     } else {
       RWLock::RLocker owner_lock(ictx->owner_lock);
       C_SaferCond cond_ctx;
-      r = snap_remove_helper(ictx, &cond_ctx, snap_name);
-      if (r < 0) {
-        return r;
-      }
+      snap_remove_helper(ictx, &cond_ctx, snap_name);
 
       r = cond_ctx.wait();
       if (r < 0) {
@@ -787,7 +781,7 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
     return 0;
   }
 
-  int snap_remove_helper(ImageCtx *ictx, Context *ctx, const char *snap_name)
+  void snap_remove_helper(ImageCtx *ictx, Context *ctx, const char *snap_name)
   {
     assert(ictx->owner_lock.is_locked());
     {
@@ -802,7 +796,8 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
 
     int r = ictx_check(ictx, ictx->owner_lock);
     if (r < 0) {
-      return r;
+      ctx->complete(r);
+      return;
     }
 
     uint64_t snap_id;
@@ -811,7 +806,8 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
       snap_id = ictx->get_snap_id(snap_name);
       if (snap_id == CEPH_NOSNAP) {
         lderr(ictx->cct) << "No such snapshot found." << dendl;
-        return -ENOENT;
+        ctx->complete(-ENOENT);
+        return;
       }
 
       bool is_protected;
@@ -829,7 +825,6 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
     operation::SnapshotRemoveRequest *req =
       new operation::SnapshotRemoveRequest(*ictx, ctx, snap_name, snap_id);
     req->send();
-    return 0;
   }
 
   int snap_rename(ImageCtx *ictx, const char *srcname, const char *dstname)
@@ -882,8 +877,8 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
     return 0;
   }
 
-  int snap_rename_helper(ImageCtx* ictx, Context* ctx,
-                         const uint64_t src_snap_id, const char* dst_name) {
+  void snap_rename_helper(ImageCtx* ictx, Context* ctx,
+                          const uint64_t src_snap_id, const char* dst_name) {
     assert(ictx->owner_lock.is_locked());
     if ((ictx->features & RBD_FEATURE_JOURNALING) != 0) {
       assert(!ictx->image_watcher->is_lock_supported() ||
@@ -894,13 +889,13 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
 
     int r = ictx_check(ictx, ictx->owner_lock);
     if (r < 0) {
-      return r;
+      ctx->complete(r);
+      return;
     }
 
     operation::SnapshotRenameRequest *req =
       new operation::SnapshotRenameRequest(*ictx, ctx, src_snap_id, dst_name);
     req->send();
-    return 0;
   }
 
   int snap_protect(ImageCtx *ictx, const char *snap_name)
@@ -2102,8 +2097,8 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
     return r;
   }
 
-  int async_resize(ImageCtx *ictx, Context *ctx, uint64_t size,
-                  ProgressContext &prog_ctx)
+  void async_resize(ImageCtx *ictx, Context *ctx, uint64_t size,
+                    ProgressContext &prog_ctx)
   {
     assert(ictx->owner_lock.is_locked());
     assert(!ictx->image_watcher->is_lock_supported() ||
@@ -2117,26 +2112,20 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
 
     int r = ictx_check(ictx, ictx->owner_lock);
     if (r < 0) {
-      return r;
+      ctx->complete(r);
+      return;
     }
 
     {
       RWLock::RLocker snap_locker(ictx->snap_lock);
       if (ictx->snap_id != CEPH_NOSNAP || ictx->read_only) {
-        return -EROFS;
+        ctx->complete(-EROFS);
+        return;
       }
     }
 
-    async_resize_helper(ictx, ctx, size, prog_ctx);
-    return 0;
-  }
-
-  void async_resize_helper(ImageCtx *ictx, Context *ctx, uint64_t new_size,
-                           ProgressContext& prog_ctx)
-  {
-    assert(ictx->owner_lock.is_locked());
     operation::ResizeRequest *req = new operation::ResizeRequest(
-      *ictx, ctx, new_size, prog_ctx);
+      *ictx, ctx, size, prog_ctx);
     req->send();
   }
 
@@ -2938,7 +2927,7 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
     return 0;
   }
 
-  int async_flatten(ImageCtx *ictx, Context *ctx, ProgressContext &prog_ctx)
+  void async_flatten(ImageCtx *ictx, Context *ctx, ProgressContext &prog_ctx)
   {
     assert(ictx->owner_lock.is_locked());
     assert(!ictx->image_watcher->is_lock_supported() ||
@@ -2951,7 +2940,8 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
     // ictx_check also updates parent data
     if ((r = ictx_check(ictx, ictx->owner_lock)) < 0) {
       lderr(cct) << "ictx_check failed" << dendl;
-      return r;
+      ctx->complete(r);
+      return;
     }
 
     uint64_t object_size;
@@ -2964,17 +2954,20 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
       RWLock::RLocker l2(ictx->parent_lock);
 
       if (ictx->read_only) {
-        return -EROFS;
+        ctx->complete(-EROFS);
+        return;
       }
 
       // can't flatten a non-clone
       if (ictx->parent_md.spec.pool_id == -1) {
        lderr(cct) << "image has no parent" << dendl;
-       return -EINVAL;
+        ctx->complete(-EINVAL);
+       return;
       }
       if (ictx->snap_id != CEPH_NOSNAP) {
        lderr(cct) << "snapshots cannot be flattened" << dendl;
-       return -EROFS;
+        ctx->complete(-EROFS);
+       return;
       }
 
       snapc = ictx->snapc;
@@ -2990,7 +2983,6 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
     operation::FlattenRequest *req = new operation::FlattenRequest(
       *ictx, ctx, object_size, overlap_objects, snapc, prog_ctx);
     req->send();
-    return 0;
   }
 
   int rebuild_object_map(ImageCtx *ictx, ProgressContext &prog_ctx) {
@@ -3017,8 +3009,8 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
     return r;
   }
 
-  int async_rebuild_object_map(ImageCtx *ictx, Context *ctx,
-                               ProgressContext &prog_ctx) {
+  void async_rebuild_object_map(ImageCtx *ictx, Context *ctx,
+                                ProgressContext &prog_ctx) {
     assert(ictx->owner_lock.is_locked());
     assert(!ictx->image_watcher->is_lock_supported() ||
           ictx->image_watcher->is_lock_owner());
@@ -3027,21 +3019,23 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
     ldout(cct, 20) << "async_rebuild_object_map " << ictx << dendl;
 
     if (ictx->read_only) {
-      return -EROFS;
+      ctx->complete(-EROFS);
+      return;
     }
     if (!ictx->test_features(RBD_FEATURE_OBJECT_MAP)) {
-      return -EINVAL;
+      ctx->complete(-EINVAL);
+      return;
     }
 
     int r = ictx_check(ictx, ictx->owner_lock);
     if (r < 0) {
-      return r;
+      ctx->complete(r);
+      return;
     }
 
     operation::RebuildObjectMapRequest *req =
       new operation::RebuildObjectMapRequest(*ictx, ctx, prog_ctx);
     req->send();
-    return 0;
   }
 
   int list_lockers(ImageCtx *ictx,

diff --git a/src/librbd/internal.h b/src/librbd/internal.h
index 0bbb908fe47c3affc01c8b7b1f9d51b0471ba1b5..2300fcc998cbee07e254e101addd72ca65d28a8d 100644 (file)
--- a/src/librbd/internal.h
+++ b/src/librbd/internal.h
@@ -126,16 +126,16 @@ namespace librbd {
             ProgressContext& prog_ctx);
   int resize(ImageCtx *ictx, uint64_t size, ProgressContext& prog_ctx);
   int snap_create(ImageCtx *ictx, const char *snap_name);
-  int snap_create_helper(ImageCtx *ictx, Context* ctx, const char *snap_name);
+  void snap_create_helper(ImageCtx *ictx, Context* ctx, const char *snap_name);
   int snap_list(ImageCtx *ictx, std::vector<snap_info_t>& snaps);
   bool snap_exists(ImageCtx *ictx, const char *snap_name);
   int snap_rollback(ImageCtx *ictx, const char *snap_name,
                    ProgressContext& prog_ctx);
   int snap_remove(ImageCtx *ictx, const char *snap_name);
-  int snap_remove_helper(ImageCtx *ictx, Context* ctx, const char *snap_name);
+  void snap_remove_helper(ImageCtx *ictx, Context* ctx, const char *snap_name);
   int snap_rename(ImageCtx *ictx, const char *srcname, const char *dstname);
-  int snap_rename_helper(ImageCtx *ictx, Context* ctx,
-                         const uint64_t src_snap_id, const char *dst_name);
+  void snap_rename_helper(ImageCtx *ictx, Context* ctx,
+                          const uint64_t src_snap_id, const char *dst_name);
   int snap_protect(ImageCtx *ictx, const char *snap_name);
   int snap_unprotect(ImageCtx *ictx, const char *snap_name);
   int snap_is_protected(ImageCtx *ictx, const char *snap_name,
@@ -197,13 +197,11 @@ namespace librbd {
   void readahead(ImageCtx *ictx,
                  const vector<pair<uint64_t,uint64_t> >& image_extents);
 
-  int async_flatten(ImageCtx *ictx, Context *ctx, ProgressContext &prog_ctx);
-  int async_resize(ImageCtx *ictx, Context *ctx, uint64_t size,
-                  ProgressContext &prog_ctx);
-  void async_resize_helper(ImageCtx *ictx, Context *ctx, uint64_t new_size,
-                           ProgressContext& prog_ctx);
-  int async_rebuild_object_map(ImageCtx *ictx, Context *ctx,
-                               ProgressContext &prog_ctx);
+  void async_flatten(ImageCtx *ictx, Context *ctx, ProgressContext &prog_ctx);
+  void async_resize(ImageCtx *ictx, Context *ctx, uint64_t size,
+                    ProgressContext &prog_ctx);
+  void async_rebuild_object_map(ImageCtx *ictx, Context *ctx,
+                                ProgressContext &prog_ctx);
 
   int flush(ImageCtx *ictx);
   int invalidate_cache(ImageCtx *ictx);