]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
rbd-mirror: fix possible race condition in error path tests 19000/head
authorJason Dillaman <dillaman@redhat.com>
Tue, 21 Nov 2017 15:08:13 +0000 (10:08 -0500)
committerJason Dillaman <dillaman@redhat.com>
Tue, 21 Nov 2017 15:09:18 +0000 (10:09 -0500)
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
src/test/rbd_mirror/test_ImageDeleter.cc
src/test/rbd_mirror/test_mock_ImageReplayer.cc
src/tools/rbd_mirror/ImageDeleter.cc
src/tools/rbd_mirror/ImageDeleter.h
src/tools/rbd_mirror/ImageReplayer.cc

index 281237f607d9dd4cc1d36bb51eb13350f12db20c..6ee55d7c08369923bf07b4558322ea05a52485ca 100644 (file)
@@ -229,7 +229,8 @@ int64_t TestImageDeleter::m_local_pool_id;
 
 
 TEST_F(TestImageDeleter, Delete_NonPrimary_Image) {
-  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false);
+  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false,
+                                   nullptr);
 
   C_SaferCond ctx;
   m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
@@ -246,7 +247,8 @@ TEST_F(TestImageDeleter, Delete_Split_Brain_Image) {
   promote_image();
   demote_image();
 
-  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, true);
+  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, true,
+                                   nullptr);
 
   C_SaferCond ctx;
   m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
@@ -262,11 +264,9 @@ TEST_F(TestImageDeleter, Delete_Split_Brain_Image) {
 TEST_F(TestImageDeleter, Fail_Delete_Primary_Image) {
   promote_image();
 
-  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false);
-
   C_SaferCond ctx;
-  m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
-                                         &ctx);
+  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false,
+                                   &ctx);
   EXPECT_EQ(-EPERM, ctx.wait());
 
   ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
@@ -277,11 +277,9 @@ TEST_F(TestImageDeleter, Fail_Delete_Orphan_Image) {
   promote_image();
   demote_image();
 
-  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false);
-
   C_SaferCond ctx;
-  m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
-                                         &ctx);
+  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false,
+                                   &ctx);
   EXPECT_EQ(-EPERM, ctx.wait());
 
   ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
@@ -291,7 +289,8 @@ TEST_F(TestImageDeleter, Fail_Delete_Orphan_Image) {
 TEST_F(TestImageDeleter, Delete_Image_With_Child) {
   create_snapshot();
 
-  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false);
+  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false,
+                                   nullptr);
 
   C_SaferCond ctx;
   m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
@@ -306,7 +305,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_Children) {
   create_snapshot("snap1");
   create_snapshot("snap2");
 
-  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false);
+  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false,
+                                   nullptr);
 
   C_SaferCond ctx;
   m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
@@ -320,7 +320,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_Children) {
 TEST_F(TestImageDeleter, Delete_Image_With_ProtectedChild) {
   create_snapshot("snap1", true);
 
-  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false);
+  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false,
+                                   nullptr);
 
   C_SaferCond ctx;
   m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
@@ -335,7 +336,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_ProtectedChildren) {
   create_snapshot("snap1", true);
   create_snapshot("snap2", true);
 
-  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false);
+  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false,
+                                   nullptr);
 
   C_SaferCond ctx;
   m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
@@ -349,20 +351,15 @@ TEST_F(TestImageDeleter, Delete_Image_With_ProtectedChildren) {
 TEST_F(TestImageDeleter, Delete_Image_With_Clone) {
   std::string clone_id = create_clone();
 
-  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false);
-  m_deleter->set_busy_timer_interval(0.1);
-
   C_SaferCond ctx;
-  m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
-                                         &ctx);
+  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false,
+                                   &ctx);
+  m_deleter->set_busy_timer_interval(0.1);
   EXPECT_EQ(-EBUSY, ctx.wait());
 
-  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_CLONE_IMAGE_ID,
-                                   false);
-
   C_SaferCond ctx2;
-  m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_CLONE_IMAGE_ID,
-                                         &ctx2);
+  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_CLONE_IMAGE_ID,
+                                   false, &ctx2);
   EXPECT_EQ(0, ctx2.wait());
 
   C_SaferCond ctx3;
@@ -382,7 +379,8 @@ TEST_F(TestImageDeleter, Delete_NonExistent_Image) {
   EXPECT_EQ(0, cls_client::mirror_image_set(&m_local_io_ctx, m_local_image_id,
                                             mirror_image));
 
-  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false);
+  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false,
+                                   nullptr);
 
   C_SaferCond ctx;
   m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
@@ -406,7 +404,8 @@ TEST_F(TestImageDeleter, Delete_NonExistent_Image_With_MirroringState) {
   EXPECT_EQ(0, cls_client::mirror_image_set(&m_local_io_ctx, m_local_image_id,
                                             mirror_image));
 
-  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false);
+  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false,
+                                   nullptr);
 
   C_SaferCond ctx;
   m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
@@ -422,11 +421,9 @@ TEST_F(TestImageDeleter, Delete_NonExistent_Image_With_MirroringState) {
 TEST_F(TestImageDeleter, Delete_NonExistent_Image_Without_MirroringState) {
   remove_image();
 
-  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false);
-
   C_SaferCond ctx;
-  m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
-                                         &ctx);
+  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false,
+                                   &ctx);
   EXPECT_EQ(-ENOENT, ctx.wait());
 
   ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
@@ -440,11 +437,9 @@ TEST_F(TestImageDeleter, Fail_Delete_NonPrimary_Image) {
                                 false);
   EXPECT_EQ(0, ictx->state->open(false));
 
-  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false);
-
   C_SaferCond ctx;
-  m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
-                                         &ctx);
+  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false,
+                                   &ctx);
   EXPECT_EQ(-EBUSY, ctx.wait());
 
   EXPECT_EQ(0, ictx->state->close());
@@ -456,11 +451,9 @@ TEST_F(TestImageDeleter, Retry_Failed_Deletes) {
                                 false);
   EXPECT_EQ(0, ictx->state->open(false));
 
-  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false);
-
   C_SaferCond ctx;
-  m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
-                                         &ctx);
+  m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false,
+                                   &ctx);
   EXPECT_EQ(-EBUSY, ctx.wait());
 
   EXPECT_EQ(0, ictx->state->close());
index 0821da96a120ff57b1f33ff890df6a420659e019..3df247be94a46b6c40511d916eee64de1fd2b964 100644 (file)
@@ -85,7 +85,8 @@ struct Threads<librbd::MockTestImageCtx> {
 
 template <>
 struct ImageDeleter<librbd::MockTestImageCtx> {
-  MOCK_METHOD3(schedule_image_delete, void(IoCtxRef, const std::string&, bool));
+  MOCK_METHOD4(schedule_image_delete, void(IoCtxRef, const std::string&, bool,
+                                           Context*));
   MOCK_METHOD4(wait_for_scheduled_deletion,
                void(int64_t, const std::string&, Context*, bool));
   MOCK_METHOD2(cancel_waiter, void(int64_t, const std::string&));
@@ -399,7 +400,7 @@ public:
                                     const std::string& global_image_id,
                                     bool ignore_orphan) {
     EXPECT_CALL(mock_image_deleter,
-                schedule_image_delete(_, global_image_id, ignore_orphan));
+                schedule_image_delete(_, global_image_id, ignore_orphan, nullptr));
   }
 
   bufferlist encode_tag_data(const librbd::journal::TagData &tag_data) {
index 23a53793bdd4c95f5deda8e97e09068a8aa1f1a0..19a3c79aa84820b8b82c8eb31752ea72050f863e 100644 (file)
@@ -165,11 +165,18 @@ ImageDeleter<I>::~ImageDeleter() {
 template <typename I>
 void ImageDeleter<I>::schedule_image_delete(IoCtxRef local_io_ctx,
                                             const std::string& global_image_id,
-                                            bool ignore_orphaned) {
+                                            bool ignore_orphaned,
+                                            Context *on_delete) {
   int64_t local_pool_id = local_io_ctx->get_id();
   dout(5) << "local_pool_id=" << local_pool_id << ", "
           << "global_image_id=" << global_image_id << dendl;
 
+  if (on_delete != nullptr) {
+    on_delete = new FunctionContext([this, on_delete](int r) {
+        m_work_queue->queue(on_delete, r);
+      });
+  }
+
   {
     Mutex::Locker locker(m_lock);
     auto del_info = find_delete_info(local_pool_id, global_image_id);
@@ -179,11 +186,17 @@ void ImageDeleter<I>::schedule_image_delete(IoCtxRef local_io_ctx,
       if (ignore_orphaned) {
         del_info->ignore_orphaned = true;
       }
+
+      if (del_info->on_delete != nullptr) {
+        del_info->on_delete->complete(-ESTALE);
+      }
+      del_info->on_delete = on_delete;
       return;
     }
 
     m_delete_queue.emplace_back(new DeleteInfo(local_pool_id, global_image_id,
-                                               local_io_ctx, ignore_orphaned));
+                                               local_io_ctx, ignore_orphaned,
+                                               on_delete));
   }
   remove_images();
 }
index b2c87dfe906f273dc1ebc2f4c4d6cc6416d45dc7..83a6aba21796f18eebad0926978d2b706f5808a0 100644 (file)
@@ -51,7 +51,8 @@ public:
 
   void schedule_image_delete(IoCtxRef local_io_ctx,
                              const std::string& global_image_id,
-                             bool ignore_orphaned);
+                             bool ignore_orphaned,
+                             Context *on_finish);
   void wait_for_scheduled_deletion(int64_t local_pool_id,
                                    const std::string &global_image_id,
                                    Context *ctx,
@@ -76,22 +77,24 @@ private:
     std::string global_image_id;
     IoCtxRef local_io_ctx;
     bool ignore_orphaned = false;
+    Context *on_delete = nullptr;
 
     image_deleter::ErrorResult error_result = {};
     int error_code = 0;
     utime_t retry_time = {};
     int retries = 0;
     bool notify_on_failed_retry = true;
-    Context *on_delete = nullptr;
 
     DeleteInfo(int64_t local_pool_id, const std::string& global_image_id)
       : local_pool_id(local_pool_id), global_image_id(global_image_id) {
     }
 
     DeleteInfo(int64_t local_pool_id, const std::string& global_image_id,
-               IoCtxRef local_io_ctx, bool ignore_orphaned)
+               IoCtxRef local_io_ctx, bool ignore_orphaned,
+               Context *on_delete)
       : local_pool_id(local_pool_id), global_image_id(global_image_id),
-        local_io_ctx(local_io_ctx), ignore_orphaned(ignore_orphaned) {
+        local_io_ctx(local_io_ctx), ignore_orphaned(ignore_orphaned),
+        on_delete(on_delete) {
     }
 
     inline bool operator==(const DeleteInfo& delete_info) const {
index 98094091d4242238b374738bd31fee35934781fe..64c4ee3f047f1c49498dd2e29d286b24ae10de0d 100644 (file)
@@ -1657,9 +1657,8 @@ void ImageReplayer<I>::handle_shut_down(int r) {
       delete_requested = true;
     }
     if (delete_requested || m_resync_requested) {
-      m_image_deleter->schedule_image_delete(m_local_ioctx,
-                                             m_global_image_id,
-                                             m_resync_requested);
+      m_image_deleter->schedule_image_delete(m_local_ioctx, m_global_image_id,
+                                             m_resync_requested, nullptr);
 
       m_local_image_id = "";
       m_resync_requested = false;