librbd: limit the number of ENOENT retries in RefreshRequest

If the image header is corrupt, ENOENT error may be persistent.  Avoid
an infinite loop in that case.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>

diff --git a/src/librbd/image/RefreshRequest.cc b/src/librbd/image/RefreshRequest.cc
index 9021eec8cb4f449684f9f427de0f7df22df6c1ee..19a57b9a511d0c9cfdc55ba96e2242c8da9cf588 100644 (file)
--- a/src/librbd/image/RefreshRequest.cc
+++ b/src/librbd/image/RefreshRequest.cc
@@ -805,8 +805,8 @@ Context *RefreshRequest<I>::handle_v2_get_snapshots(int *result) {
     }
   }
 
-  if (*result == -ENOENT) {
-    ldout(cct, 10) << "out-of-sync snapshot state detected" << dendl;
+  if (*result == -ENOENT && m_enoent_retries++ < MAX_ENOENT_RETRIES) {
+    ldout(cct, 10) << "out-of-sync snapshot state detected, retrying" << dendl;
     send_v2_get_mutable_metadata();
     return nullptr;
   } else if (m_legacy_snapshot == LEGACY_SNAPSHOT_DISABLED &&

diff --git a/src/librbd/image/RefreshRequest.h b/src/librbd/image/RefreshRequest.h
index b7b877dd3c2aaf016fb84e053157d83814b77f0d..ab7b5089295d1fda6c270d4fe21134ed902cfe61 100644 (file)
--- a/src/librbd/image/RefreshRequest.h
+++ b/src/librbd/image/RefreshRequest.h
@@ -27,6 +27,8 @@ template<typename> class RefreshParentRequest;
 template<typename ImageCtxT = ImageCtx>
 class RefreshRequest {
 public:
+  static constexpr int MAX_ENOENT_RETRIES = 10;
+
   static RefreshRequest *create(ImageCtxT &image_ctx, bool acquiring_lock,
                                 bool skip_open_parent, Context *on_finish) {
     return new RefreshRequest(image_ctx, acquiring_lock, skip_open_parent,
@@ -144,6 +146,8 @@ private:
   bool m_legacy_parent = false;
   LegacySnapshot m_legacy_snapshot = LEGACY_SNAPSHOT_DISABLED;
 
+  int m_enoent_retries = 0;
+
   uint8_t m_order = 0;
   uint64_t m_size = 0;
   uint64_t m_features = 0;

diff --git a/src/test/librbd/image/test_mock_RefreshRequest.cc b/src/test/librbd/image/test_mock_RefreshRequest.cc
index 96b8fe306737b5ae4dfe174c268d4c4c81c4cf5a..261604c43375cc3c310a62e0dc4a0d5a05c43751 100644 (file)
--- a/src/test/librbd/image/test_mock_RefreshRequest.cc
+++ b/src/test/librbd/image/test_mock_RefreshRequest.cc
@@ -722,7 +722,6 @@ TEST_F(TestMockImageRefreshRequest, SuccessLegacySnapshotNoTimestampV2) {
   ASSERT_EQ(0, ctx.wait());
 }
 
-
 TEST_F(TestMockImageRefreshRequest, SuccessSetSnapshotV2) {
   REQUIRE_FORMAT_V2();
 
@@ -765,6 +764,38 @@ TEST_F(TestMockImageRefreshRequest, SuccessSetSnapshotV2) {
   ASSERT_EQ(0, ctx.wait());
 }
 
+TEST_F(TestMockImageRefreshRequest, SnapshotV2EnoentRetriesLimit) {
+  REQUIRE_FORMAT_V2();
+
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+  ASSERT_EQ(0, snap_create(*ictx, "snap"));
+
+  MockRefreshImageCtx mock_image_ctx(*ictx);
+  MockGetMetadataRequest mock_get_metadata_request;
+  expect_op_work_queue(mock_image_ctx);
+  expect_test_features(mock_image_ctx);
+
+  InSequence seq;
+  for (int i = 0; i < RefreshRequest<>::MAX_ENOENT_RETRIES + 1; ++i) {
+    expect_get_mutable_metadata(mock_image_ctx, ictx->features, 0);
+    expect_get_parent(mock_image_ctx, 0);
+    expect_get_metadata(mock_image_ctx, mock_get_metadata_request,
+                        mock_image_ctx.header_oid, {}, 0);
+    expect_get_metadata(mock_image_ctx, mock_get_metadata_request, RBD_INFO, {},
+                        0);
+    expect_apply_metadata(mock_image_ctx, 0);
+    expect_get_group(mock_image_ctx, 0);
+    expect_get_snapshots(mock_image_ctx, false, -ENOENT);
+  }
+
+  C_SaferCond ctx;
+  auto req = new MockRefreshRequest(mock_image_ctx, false, false, &ctx);
+  req->send();
+
+  ASSERT_EQ(-ENOENT, ctx.wait());
+}
+
 TEST_F(TestMockImageRefreshRequest, SuccessChild) {
   REQUIRE_FEATURE(RBD_FEATURE_LAYERING);