From: Jason Dillaman <dillaman@redhat.com>
Date: Wed, 18 Mar 2015 15:51:47 +0000 (-0400)
Subject: librbd: use generic helper for issuing async requests
X-Git-Tag: v0.94.2~18^2~3
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=ec0bd1dea526e04333d8059421666dcd2a59044e;p=ceph.git

librbd: use generic helper for issuing async requests

resize, flatten, and rebuild object map now use the same
bootstrap code for sending the request to the remote lock owner
or executing the request locally.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit 18fd6ca7f59d5545f0bb0b0e899d0739639ce104)

Conflicts:
	src/librbd/internal.cc
---

diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc
index b5f7f60f2700..cdda22d779b5 100644
--- a/src/librbd/ImageWatcher.cc
+++ b/src/librbd/ImageWatcher.cc
@@ -895,7 +895,8 @@ void ImageWatcher::handle_payload(const SnapCreatePayload &payload,
   if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) {
     ldout(m_image_ctx.cct, 10) << "remote snap_create request: "
 			       << payload.snap_name << dendl;
-    int r = librbd::snap_create(&m_image_ctx, payload.snap_name.c_str(), false);
+    int r = librbd::snap_create_helper(&m_image_ctx, NULL,
+                                       payload.snap_name.c_str());
 
     ::encode(ResponseMessage(r), *out);
   }
diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index cf524e48a3c5..664927c0f21b 100644
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -54,6 +54,86 @@ using librados::IoCtx;
 using librados::Rados;
 
 namespace librbd {
+
+namespace {
+
+int prepare_image_update(ImageCtx *ictx) {
+  assert(ictx->owner_lock.is_locked() && !ictx->owner_lock.is_wlocked());
+  if (ictx->image_watcher == NULL) {
+    return -EROFS;
+  } else if (!ictx->image_watcher->is_lock_supported() ||
+             ictx->image_watcher->is_lock_owner()) {
+    return 0;
+  }
+
+  // need to upgrade to a write lock
+  int r = 0;
+  bool acquired_lock = false;
+  ictx->owner_lock.put_read();
+  {
+    RWLock::WLocker l(ictx->owner_lock);
+    if (!ictx->image_watcher->is_lock_owner()) {
+      r = ictx->image_watcher->try_lock();
+      acquired_lock = ictx->image_watcher->is_lock_owner();
+    }
+  }
+  if (acquired_lock) {
+    // finish any AIO that was previously waiting on acquiring the
+    // exclusive lock
+    ictx->flush_async_operations();
+  }
+  ictx->owner_lock.get_read();
+  return r;
+}
+
+int invoke_async_request(ImageCtx *ictx, const std::string& request_type,
+                         const boost::function<int(Context*)>& local_request,
+                         const boost::function<int()>& remote_request) {
+  int r;
+  do {
+    C_SaferCond ctx;
+    {
+      RWLock::RLocker l(ictx->owner_lock);
+      {
+        RWLock::RLocker snap_locker(ictx->snap_lock);
+        if (ictx->read_only || ictx->snap_id != CEPH_NOSNAP) {
+          return -EROFS;
+        }
+      }
+
+      while (ictx->image_watcher->is_lock_supported()) {
+        r = prepare_image_update(ictx);
+        if (r < 0) {
+          return -EROFS;
+        } else if (ictx->image_watcher->is_lock_owner()) {
+          break;
+        }
+
+        r = remote_request();
+        if (r != -ETIMEDOUT && r != -ERESTART) {
+          return r;
+        }
+        ldout(ictx->cct, 5) << request_type << " timed out notifying lock owner"
+                            << dendl;
+      }
+
+      r = local_request(&ctx);
+      if (r < 0) {
+        return r;
+      }
+    }
+
+    r = ctx.wait();
+    if (r == -ERESTART) {
+      ldout(ictx->cct, 5) << request_type << " interrupted: restarting"
+                          << dendl;
+    }
+  } while (r == -ERESTART);
+  return r;
+}
+
+} // anonymous namespace
+
   const string id_obj_name(const string &name)
   {
     return RBD_ID_PREFIX + name;
@@ -442,43 +522,13 @@ namespace librbd {
     return 0;
   }
 
-  static int prepare_image_update(ImageCtx *ictx)
-  {
-    assert(ictx->owner_lock.is_locked() && !ictx->owner_lock.is_wlocked());
-    if (ictx->image_watcher == NULL) {
-      return -EROFS;
-    } else if (!ictx->image_watcher->is_lock_supported() ||
-	       ictx->image_watcher->is_lock_owner()) {
-      return 0;
-    }
-
-    // need to upgrade to a write lock
-    int r = 0;
-    bool acquired_lock = false;
-    ictx->owner_lock.put_read();
-    {
-      RWLock::WLocker l(ictx->owner_lock);
-      if (!ictx->image_watcher->is_lock_owner()) {
-	r = ictx->image_watcher->try_lock();
-        acquired_lock = ictx->image_watcher->is_lock_owner();
-      }
-    }
-    if (acquired_lock) {
-      // finish any AIO that was previously waiting on acquiring the
-      // exclusive lock
-      ictx->flush_async_operations();
-    }
-    ictx->owner_lock.get_read();
-    return r;
-  }
-
-  int snap_create(ImageCtx *ictx, const char *snap_name, bool notify)
+  int snap_create(ImageCtx *ictx, const char *snap_name)
   {
-    assert(ictx->owner_lock.is_locked());
     ldout(ictx->cct, 20) << "snap_create " << ictx << " " << snap_name << dendl;
 
-    if (ictx->read_only)
+    if (ictx->read_only) {
       return -EROFS;
+    }
 
     int r = ictx_check(ictx);
     if (r < 0)
@@ -491,45 +541,51 @@ namespace librbd {
       }
     }
 
-    bool lock_owner = false;
-    while (ictx->image_watcher->is_lock_supported()) {
-      r = prepare_image_update(ictx);
-      if (r < 0) {
-	return -EROFS;
-      } else if (ictx->image_watcher->is_lock_owner()) {
-	lock_owner = true;
-	break;
-      }
+    r = invoke_async_request(ictx, "snap_create",
+                             boost::bind(&snap_create_helper, ictx, _1,
+                                         snap_name),
+                             boost::bind(&ImageWatcher::notify_snap_create,
+                                         ictx->image_watcher, snap_name));
+    if (r < 0 && r != -EEXIST) {
+      return r;
+    }
 
-      r = ictx->image_watcher->notify_snap_create(snap_name);
-      if (r == 0 || r == -EEXIST) {
-        notify_change(ictx->md_ctx, ictx->header_oid, ictx);
-        return 0;
-      } else if (r != -ETIMEDOUT) {
-	return r;
-      }
-      ldout(ictx->cct, 5) << "snap_create timed out notifying lock owner" << dendl;
+    ictx->perfcounter->inc(l_librbd_snap_create);
+    notify_change(ictx->md_ctx, ictx->header_oid, ictx);
+    return 0;
+  }
+
+  int 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());
+
+    ldout(ictx->cct, 20) << "snap_create_helper " << ictx << " " << snap_name
+                         << dendl;
+
+    int r = ictx_check(ictx);
+    if (r < 0) {
+      return r;
     }
 
-    RWLock::WLocker l2(ictx->md_lock);
+    RWLock::WLocker md_locker(ictx->md_lock);
     r = _flush(ictx);
     if (r < 0) {
       return r;
     }
 
     do {
-      r = add_snap(ictx, snap_name, lock_owner);
+      r = add_snap(ictx, snap_name);
     } while (r == -ESTALE);
 
     if (r < 0) {
       return r;
     }
 
-    if (notify) {
-      notify_change(ictx->md_ctx, ictx->header_oid, ictx);
+    if (ctx != NULL) {
+      ctx->complete(0);
     }
-
-    ictx->perfcounter->inc(l_librbd_snap_create);
     return 0;
   }
 
@@ -1649,42 +1705,12 @@ reprotect_and_return_err:
     }
 
     uint64_t request_id = ictx->async_request_seq.inc();
-    do {
-      C_SaferCond ctx;
-      {
-	RWLock::RLocker l(ictx->owner_lock);
-	while (ictx->image_watcher->is_lock_supported()) {
-	  r = prepare_image_update(ictx);
-	  if (r < 0) {
-	    return -EROFS;
-	  } else if (ictx->image_watcher->is_lock_owner()) {
-	    break;
-	  }
-
-          RWLock::RLocker snap_locker(ictx->snap_lock);
-          if (ictx->snap_id != CEPH_NOSNAP || ictx->read_only) {
-            return -EROFS;
-          }
-
-	  r = ictx->image_watcher->notify_resize(request_id, size, prog_ctx);
-	  if (r != -ETIMEDOUT && r != -ERESTART) {
-	    return r;
-	  }
-	  ldout(ictx->cct, 5) << "resize timed out notifying lock owner"
-                              << dendl;
-	}
-
-	r = async_resize(ictx, &ctx, size, prog_ctx);
-	if (r < 0) {
-	  return r;
-	}
-      }
-
-      r = ctx.wait();
-      if (r == -ERESTART) {
-	ldout(ictx->cct, 5) << "resize interrupted: restarting" << dendl;
-      }
-    } while (r == -ERESTART);
+    r = invoke_async_request(ictx, "resize",
+                             boost::bind(&async_resize, ictx, _1, size,
+                                         boost::ref(prog_ctx)),
+                             boost::bind(&ImageWatcher::notify_resize,
+                                         ictx->image_watcher, request_id, size,
+                                         boost::ref(prog_ctx)));
 
     ictx->perfcounter->inc(l_librbd_resize);
     notify_change(ictx->md_ctx, ictx->header_oid, ictx);
@@ -1765,12 +1791,17 @@ reprotect_and_return_err:
   }
 
 
-  int add_snap(ImageCtx *ictx, const char *snap_name, bool lock_owner)
+  int add_snap(ImageCtx *ictx, const char *snap_name)
   {
     assert(ictx->owner_lock.is_locked());
     assert(ictx->md_lock.is_wlocked());
-    uint64_t snap_id;
 
+    bool lock_owner = ictx->image_watcher->is_lock_owner();
+    if (ictx->image_watcher->is_lock_supported()) {
+      assert(lock_owner);
+    }
+
+    uint64_t snap_id;
     int r = ictx->md_ctx.selfmanaged_snap_create(&snap_id);
     if (r < 0) {
       lderr(ictx->cct) << "failed to create snap id: " << cpp_strerror(-r)
@@ -1783,7 +1814,7 @@ reprotect_and_return_err:
 				       snap_id, snap_name);
     } else {
       librados::ObjectWriteOperation op;
-      if (ictx->image_watcher->is_lock_supported()) {
+      if (lock_owner) {
 	ictx->image_watcher->assert_header_locked(&op);
       }
       cls_client::snapshot_add(&op, snap_id, snap_name);
@@ -2485,45 +2516,13 @@ reprotect_and_return_err:
       return r;
     }
 
-    {
-      RWLock::RLocker snap_locker(ictx->snap_lock);
-      if (ictx->read_only || ictx->snap_id != CEPH_NOSNAP) {
-        return -EROFS;
-      }
-    }
-
     uint64_t request_id = ictx->async_request_seq.inc();
-    do {
-      C_SaferCond ctx;
-      {
-        RWLock::RLocker l(ictx->owner_lock);
-        while (ictx->image_watcher->is_lock_supported()) {
-          r = prepare_image_update(ictx);
-          if (r < 0) {
-            return -EROFS;
-          } else if (ictx->image_watcher->is_lock_owner()) {
-	    break;
-	  }
-
-          r = ictx->image_watcher->notify_flatten(request_id, prog_ctx);
-          if (r != -ETIMEDOUT && r != -ERESTART) {
-            return r;
-          }
-          ldout(ictx->cct, 5) << "flatten timed out notifying lock owner"
-                              << dendl;
-        }
-
-        r = async_flatten(ictx, &ctx, prog_ctx);
-        if (r < 0) {
-	  return r;
-        }
-      }
-
-      r = ctx.wait();
-      if (r == -ERESTART) {
-	ldout(ictx->cct, 5) << "flatten interrupted: restarting" << dendl;
-      }
-    } while (r == -ERESTART);
+    r = invoke_async_request(ictx, "flatten",
+                             boost::bind(&async_flatten, ictx, _1,
+                                         boost::ref(prog_ctx)),
+                             boost::bind(&ImageWatcher::notify_flatten,
+                                         ictx->image_watcher, request_id,
+                                         boost::ref(prog_ctx)));
 
     notify_change(ictx->md_ctx, ictx->header_oid, ictx);
     ldout(cct, 20) << "flatten finished" << dendl;
diff --git a/src/librbd/internal.h b/src/librbd/internal.h
index 64395ca5ea0f..eda119e15288 100644
--- a/src/librbd/internal.h
+++ b/src/librbd/internal.h
@@ -109,7 +109,8 @@ namespace librbd {
   int remove(librados::IoCtx& io_ctx, const char *imgname,
 	     ProgressContext& prog_ctx);
   int resize(ImageCtx *ictx, uint64_t size, ProgressContext& prog_ctx);
-  int snap_create(ImageCtx *ictx, const char *snap_name, bool notify);
+  int snap_create(ImageCtx *ictx, const char *snap_name);
+  int 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,
@@ -119,7 +120,7 @@ namespace librbd {
   int snap_unprotect(ImageCtx *ictx, const char *snap_name);
   int snap_is_protected(ImageCtx *ictx, const char *snap_name,
 			bool *is_protected);
-  int add_snap(ImageCtx *ictx, const char *snap_name, bool lock_owner);
+  int add_snap(ImageCtx *ictx, const char *snap_name);
   int rm_snap(ImageCtx *ictx, const char *snap_name);
   int refresh_parent(ImageCtx *ictx);
   int ictx_check(ImageCtx *ictx);
diff --git a/src/librbd/librbd.cc b/src/librbd/librbd.cc
index 2d1e56777a39..ce0d31363780 100644
--- a/src/librbd/librbd.cc
+++ b/src/librbd/librbd.cc
@@ -509,8 +509,7 @@ namespace librbd {
   {
     ImageCtx *ictx = (ImageCtx *)ctx;
     tracepoint(librbd, snap_create_enter, ictx, ictx->name.c_str(), ictx->snap_name.c_str(), ictx->read_only, snap_name);
-    RWLock::RLocker owner_locker(ictx->owner_lock);
-    int r = librbd::snap_create(ictx, snap_name, true);
+    int r = librbd::snap_create(ictx, snap_name);
     tracepoint(librbd, snap_create_exit, r);
     return r;
   }
@@ -1209,8 +1208,7 @@ extern "C" int rbd_snap_create(rbd_image_t image, const char *snap_name)
 {
   librbd::ImageCtx *ictx = (librbd::ImageCtx *)image;
   tracepoint(librbd, snap_create_enter, ictx, ictx->name.c_str(), ictx->snap_name.c_str(), ictx->read_only, snap_name);
-  RWLock::RLocker owner_locker(ictx->owner_lock);
-  int r = librbd::snap_create(ictx, snap_name, true);
+  int r = librbd::snap_create(ictx, snap_name);
   tracepoint(librbd, snap_create_exit, r);
   return r;
 }
diff --git a/src/test/librbd/test_internal.cc b/src/test/librbd/test_internal.cc
index a6d1bcf529f2..aac4d4dcc719 100644
--- a/src/test/librbd/test_internal.cc
+++ b/src/test/librbd/test_internal.cc
@@ -38,10 +38,7 @@ public:
       return r;
     }
 
-    {
-      RWLock::RLocker l(ictx->owner_lock);
-      r = librbd::snap_create(ictx, snap_name, true);
-    }
+    r = librbd::snap_create(ictx, snap_name);
     if (r < 0) {
       return r;
     }
@@ -116,10 +113,7 @@ TEST_F(TestInternal, SnapCreateLocksImage) {
   librbd::ImageCtx *ictx;
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
 
-  {
-    RWLock::RLocker l(ictx->owner_lock);
-    ASSERT_EQ(0, librbd::snap_create(ictx, "snap1", true));
-  }
+  ASSERT_EQ(0, librbd::snap_create(ictx, "snap1"));
   BOOST_SCOPE_EXIT( (ictx) ) {
     ASSERT_EQ(0, librbd::snap_remove(ictx, "snap1"));
   } BOOST_SCOPE_EXIT_END;
@@ -136,8 +130,7 @@ TEST_F(TestInternal, SnapCreateFailsToLockImage) {
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
   ASSERT_EQ(0, lock_image(*ictx, LOCK_EXCLUSIVE, "manually locked"));
 
-  RWLock::RLocker l(ictx->owner_lock);
-  ASSERT_EQ(-EROFS, librbd::snap_create(ictx, "snap1", true));
+  ASSERT_EQ(-EROFS, librbd::snap_create(ictx, "snap1"));
 }
 
 TEST_F(TestInternal, SnapRollbackLocksImage) {