librbd: ignore cache busy errors when shrinking an image

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

diff --git a/src/librbd/operation/ResizeRequest.cc b/src/librbd/operation/ResizeRequest.cc
index 86cb8a85d3bf03003b5e3398481c65af7aaf228a..67207d95d8f8c7a8f4e4f632d6e3fd2834741b95 100644 (file)
--- a/src/librbd/operation/ResizeRequest.cc
+++ b/src/librbd/operation/ResizeRequest.cc
@@ -179,6 +179,38 @@ Context *ResizeRequest<I>::handle_trim_image(int *result) {
   return nullptr;
 }
 
+template <typename I>
+void ResizeRequest<I>::send_flush_cache() {
+  I &image_ctx = this->m_image_ctx;
+  if (image_ctx.object_cacher == nullptr) {
+    send_trim_image();
+    return;
+  }
+
+  CephContext *cct = image_ctx.cct;
+  ldout(cct, 5) << this << " " << __func__ << dendl;
+
+  RWLock::RLocker owner_locker(image_ctx.owner_lock);
+  image_ctx.flush_cache(create_async_context_callback(
+    image_ctx, create_context_callback<
+      ResizeRequest<I>, &ResizeRequest<I>::handle_flush_cache>(this)));
+}
+
+template <typename I>
+Context *ResizeRequest<I>::handle_flush_cache(int *result) {
+  I &image_ctx = this->m_image_ctx;
+  CephContext *cct = image_ctx.cct;
+  ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl;
+
+  if (*result < 0) {
+    lderr(cct) << "failed to flush cache: " << cpp_strerror(*result) << dendl;
+    return this->create_context_finisher(*result);
+  }
+
+  send_invalidate_cache();
+  return nullptr;
+}
+
 template <typename I>
 void ResizeRequest<I>::send_invalidate_cache() {
   I &image_ctx = this->m_image_ctx;
@@ -199,7 +231,10 @@ Context *ResizeRequest<I>::handle_invalidate_cache(int *result) {
   CephContext *cct = image_ctx.cct;
   ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl;
 
-  if (*result < 0) {
+  // ignore busy error -- writeback was successfully flushed so we might be
+  // wasting some cache space for trimmed objects, but they will get purged
+  // eventually. Most likely cause of the issue was a in-flight cache read
+  if (*result < 0 && *result != -EBUSY) {
     lderr(cct) << "failed to invalidate cache: " << cpp_strerror(*result)
                << dendl;
     return this->create_context_finisher(*result);
@@ -222,7 +257,7 @@ Context *ResizeRequest<I>::send_grow_object_map() {
   if (m_original_size == m_new_size) {
     return this->create_context_finisher(0);
   } else if (m_new_size < m_original_size) {
-    send_invalidate_cache();
+    send_flush_cache();
     return nullptr;
   }
 

diff --git a/src/librbd/operation/ResizeRequest.h b/src/librbd/operation/ResizeRequest.h
index 394a89e3ed87a2216c3828177a27317767b9cde7..7b9ff6d8eb0f0057d4dbc01673900bff6e371943 100644 (file)
--- a/src/librbd/operation/ResizeRequest.h
+++ b/src/librbd/operation/ResizeRequest.h
@@ -80,7 +80,10 @@ private:
    *    |           STATE_UPDATE_HEADER ----------------------------\
    *    |                                                           |
    *    | (shrink)                                                  |
-   *    |\--------> STATE_INVALIDATE_CACHE                          |
+   *    |\--------> STATE_FLUSH_CACHE                               |
+   *    |                 |                                         |
+   *    |                 v                                         |
+   *    |           STATE_INVALIDATE_CACHE                          |
    *    |                 |                                         |
    *    |                 v                                         |
    *    |           STATE_TRIM_IMAGE                                |
@@ -121,6 +124,9 @@ private:
   Context *send_append_op_event();
   Context *handle_append_op_event(int *result);
 
+  void send_flush_cache();
+  Context *handle_flush_cache(int *result);
+
   void send_invalidate_cache();
   Context *handle_invalidate_cache(int *result);
 

diff --git a/src/test/librbd/mock/MockImageCtx.h b/src/test/librbd/mock/MockImageCtx.h
index ffccf95f9ee824465d277183d20944f13fc19d7f..a0c03d528ef1f048fb7dd1a45ce5c76b60606087 100644 (file)
--- a/src/test/librbd/mock/MockImageCtx.h
+++ b/src/test/librbd/mock/MockImageCtx.h
@@ -158,6 +158,7 @@ struct MockImageCtx {
   MOCK_METHOD1(flush_async_operations, void(Context *));
   MOCK_METHOD1(flush_copyup, void(Context *));
 
+  MOCK_METHOD1(flush_cache, void(Context *));
   MOCK_METHOD1(invalidate_cache, void(Context *));
   MOCK_METHOD1(shut_down_cache, void(Context *));
 

diff --git a/src/test/librbd/operation/test_mock_ResizeRequest.cc b/src/test/librbd/operation/test_mock_ResizeRequest.cc
index 4f3777bce85d094097736f0992cf5106a05b5557..17879ecddf3d450fedfb153ab597054bf5105219 100644 (file)
--- a/src/test/librbd/operation/test_mock_ResizeRequest.cc
+++ b/src/test/librbd/operation/test_mock_ResizeRequest.cc
@@ -116,6 +116,11 @@ public:
                   .WillOnce(FinishRequest(&mock_trim_request, r, &mock_image_ctx));
   }
 
+  void expect_flush_cache(MockImageCtx &mock_image_ctx, int r) {
+    EXPECT_CALL(mock_image_ctx, flush_cache(_)).WillOnce(CompleteContext(r, NULL));
+    expect_op_work_queue(mock_image_ctx);
+  }
+
   void expect_invalidate_cache(MockImageCtx &mock_image_ctx, int r) {
     EXPECT_CALL(mock_image_ctx, invalidate_cache(_))
                   .WillOnce(CompleteContext(r, NULL));
@@ -203,6 +208,7 @@ TEST_F(TestMockOperationResizeRequest, ShrinkSuccess) {
   expect_unblock_writes(mock_image_ctx);
 
   MockTrimRequest mock_trim_request;
+  expect_flush_cache(mock_image_ctx, 0);
   expect_invalidate_cache(mock_image_ctx, 0);
   expect_trim(mock_image_ctx, mock_trim_request, 0);
   expect_block_writes(mock_image_ctx, 0);
@@ -264,12 +270,35 @@ TEST_F(TestMockOperationResizeRequest, TrimError) {
   expect_unblock_writes(mock_image_ctx);
 
   MockTrimRequest mock_trim_request;
-  expect_invalidate_cache(mock_image_ctx, 0);
+  expect_flush_cache(mock_image_ctx, 0);
+  expect_invalidate_cache(mock_image_ctx, -EBUSY);
   expect_trim(mock_image_ctx, mock_trim_request, -EINVAL);
   expect_commit_op_event(mock_image_ctx, -EINVAL);
   ASSERT_EQ(-EINVAL, when_resize(mock_image_ctx, ictx->size / 2, true, 0, false));
 }
 
+TEST_F(TestMockOperationResizeRequest, FlushCacheError) {
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  MockImageCtx mock_image_ctx(*ictx);
+  MockExclusiveLock mock_exclusive_lock;
+  MockJournal mock_journal;
+  MockObjectMap mock_object_map;
+  initialize_features(ictx, mock_image_ctx, mock_exclusive_lock, mock_journal,
+                      mock_object_map);
+
+  InSequence seq;
+  expect_block_writes(mock_image_ctx, 0);
+  expect_append_op_event(mock_image_ctx, true, 0);
+  expect_unblock_writes(mock_image_ctx);
+
+  MockTrimRequest mock_trim_request;
+  expect_flush_cache(mock_image_ctx, -EINVAL);
+  expect_commit_op_event(mock_image_ctx, -EINVAL);
+  ASSERT_EQ(-EINVAL, when_resize(mock_image_ctx, ictx->size / 2, true, 0, false));
+}
+
 TEST_F(TestMockOperationResizeRequest, InvalidateCacheError) {
   librbd::ImageCtx *ictx;
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
@@ -287,6 +316,7 @@ TEST_F(TestMockOperationResizeRequest, InvalidateCacheError) {
   expect_unblock_writes(mock_image_ctx);
 
   MockTrimRequest mock_trim_request;
+  expect_flush_cache(mock_image_ctx, 0);
   expect_invalidate_cache(mock_image_ctx, -EINVAL);
   expect_commit_op_event(mock_image_ctx, -EINVAL);
   ASSERT_EQ(-EINVAL, when_resize(mock_image_ctx, ictx->size / 2, true, 0, false));