librbd: fix potential race when notify quiesce fails

In SnapshotCreateRequest, handle_notify_quiesce could return
earlier than notify_quiesce. As the result m_request_id was not
properly set at the moment when notify_unquiesce was called.

Signed-off-by: Mykola Golub <mgolub@suse.com>

diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc
index 33bf2e7a0d836e045643f8d821427cbe41c191b8..15421cb487a38d45cda53cfcf7e9619548d2e59c 100644 (file)
--- a/src/librbd/ImageWatcher.cc
+++ b/src/librbd/ImageWatcher.cc
@@ -341,21 +341,20 @@ void ImageWatcher<I>::notify_header_update(librados::IoCtx &io_ctx,
 }
 
 template <typename I>
-uint64_t ImageWatcher<I>::notify_quiesce(ProgressContext &prog_ctx,
-                                         Context *on_finish) {
-  uint64_t request_id = m_image_ctx.operations->reserve_async_request_id();
+void ImageWatcher<I>::notify_quiesce(uint64_t *request_id,
+                                     ProgressContext &prog_ctx,
+                                     Context *on_finish) {
+  *request_id = m_image_ctx.operations->reserve_async_request_id();
 
   ldout(m_image_ctx.cct, 10) << this << " " << __func__ << ": request_id="
                              << request_id << dendl;
 
-  AsyncRequestId async_request_id(get_client_id(), request_id);
+  AsyncRequestId async_request_id(get_client_id(), *request_id);
 
   auto attempts = m_image_ctx.config.template get_val<uint64_t>(
     "rbd_quiesce_notification_attempts");
 
   notify_quiesce(async_request_id, attempts, prog_ctx, on_finish);
-
-  return request_id;
 }
 
 template <typename I>

diff --git a/src/librbd/ImageWatcher.h b/src/librbd/ImageWatcher.h
index ed3db723a007818c9e87ea0cb6a7b7eb8c39ba56..0c9a634f4b81c27b945c59c2ccc141657329efcd 100644 (file)
--- a/src/librbd/ImageWatcher.h
+++ b/src/librbd/ImageWatcher.h
@@ -73,7 +73,8 @@ public:
   static void notify_header_update(librados::IoCtx &io_ctx,
                                    const std::string &oid);
 
-  uint64_t notify_quiesce(ProgressContext &prog_ctx, Context *on_finish);
+  void notify_quiesce(uint64_t *request_id, ProgressContext &prog_ctx,
+                      Context *on_finish);
   void notify_unquiesce(uint64_t request_id, Context *on_finish);
 
 private:

diff --git a/src/librbd/api/Group.cc b/src/librbd/api/Group.cc
index 6af1ed1d54f36f0d9a6de7fee1aad399cfff1f7e..2beca5adb0b9e8db71c6ce56d2af1dbefa4ae6f6 100644 (file)
--- a/src/librbd/api/Group.cc
+++ b/src/librbd/api/Group.cc
@@ -467,8 +467,8 @@ int notify_quiesce(std::vector<I*> &ictxs, ProgressContext &prog_ctx,
   for (int i = 0; i < image_count; ++i) {
     auto ictx = ictxs[i];
 
-    (*requests)[i] = ictx->image_watcher->notify_quiesce(prog_ctx,
-                                                         &on_finishes[i]);
+    ictx->image_watcher->notify_quiesce(&(*requests)[i], prog_ctx,
+                                        &on_finishes[i]);
   }
 
   int ret_code = 0;

diff --git a/src/librbd/operation/SnapshotCreateRequest.cc b/src/librbd/operation/SnapshotCreateRequest.cc
index bf3b374318450509168709be3162379b0d6dd9a7..8dc7f706e3a25c3397367f1a7abda3d104fd9bcc 100644 (file)
--- a/src/librbd/operation/SnapshotCreateRequest.cc
+++ b/src/librbd/operation/SnapshotCreateRequest.cc
@@ -65,8 +65,8 @@ void SnapshotCreateRequest<I>::send_notify_quiesce() {
   CephContext *cct = image_ctx.cct;
   ldout(cct, 5) << this << " " << __func__ << dendl;
 
-  m_request_id = image_ctx.image_watcher->notify_quiesce(
-    m_prog_ctx, create_async_context_callback(
+  image_ctx.image_watcher->notify_quiesce(
+      &m_request_id, m_prog_ctx, create_async_context_callback(
       image_ctx, create_context_callback<SnapshotCreateRequest<I>,
       &SnapshotCreateRequest<I>::handle_notify_quiesce>(this)));
 }

diff --git a/src/test/librbd/mock/MockImageWatcher.h b/src/test/librbd/mock/MockImageWatcher.h
index f58bb2ba9622553ec9032147176d038b0e6050dd..0f010564115d00c8be6c395c45f9bf8014427b33 100644 (file)
--- a/src/test/librbd/mock/MockImageWatcher.h
+++ b/src/test/librbd/mock/MockImageWatcher.h
@@ -25,7 +25,7 @@ struct MockImageWatcher {
   MOCK_METHOD0(notify_released_lock, void());
   MOCK_METHOD0(notify_request_lock, void());
 
-  MOCK_METHOD2(notify_quiesce, uint64_t(ProgressContext &, Context *));
+  MOCK_METHOD3(notify_quiesce, void(uint64_t *, ProgressContext &, Context *));
   MOCK_METHOD2(notify_unquiesce, void(uint64_t, Context *));
 };
 

diff --git a/src/test/librbd/operation/test_mock_SnapshotCreateRequest.cc b/src/test/librbd/operation/test_mock_SnapshotCreateRequest.cc
index efc9001a8857c8ededeacde92965c70f7b1e4ca6..cf56f9e298040d2efa5c75920793f8062c600f7d 100644 (file)
--- a/src/test/librbd/operation/test_mock_SnapshotCreateRequest.cc
+++ b/src/test/librbd/operation/test_mock_SnapshotCreateRequest.cc
@@ -63,10 +63,9 @@ public:
   typedef mirror::snapshot::SetImageStateRequest<MockImageCtx> MockSetImageStateRequest;
 
   void expect_notify_quiesce(MockImageCtx &mock_image_ctx, int r) {
-    EXPECT_CALL(*mock_image_ctx.image_watcher, notify_quiesce(_, _))
-      .WillOnce(DoAll(WithArg<1>(CompleteContext(
-                                   r, mock_image_ctx.image_ctx->op_work_queue)),
-                      Return(0)));
+    EXPECT_CALL(*mock_image_ctx.image_watcher, notify_quiesce(_, _, _))
+      .WillOnce(WithArg<2>(CompleteContext(
+                             r, mock_image_ctx.image_ctx->op_work_queue)));
   }
 
   void expect_block_writes(MockImageCtx &mock_image_ctx) {