librbd: notify watcher when updating image metadata

Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

diff --git a/src/librbd/Operations.cc b/src/librbd/Operations.cc
index 3fc256d72ceebf9f685604750b59d85d706e3091..15dff8224e5acac339104e70d3647d323aa83f9f 100644 (file)
--- a/src/librbd/Operations.cc
+++ b/src/librbd/Operations.cc
@@ -1406,7 +1406,9 @@ void Operations<I>::execute_metadata_set(const std::string &key,
                 << value << dendl;
 
   operation::MetadataSetRequest<I> *request =
-    new operation::MetadataSetRequest<I>(m_image_ctx, on_finish, key, value);
+    new operation::MetadataSetRequest<I>(m_image_ctx,
+                                        new C_NotifyUpdate<I>(m_image_ctx, on_finish),
+                                        key, value);
   request->send();
 }
 
@@ -1464,7 +1466,9 @@ void Operations<I>::execute_metadata_remove(const std::string &key,
   ldout(cct, 5) << this << " " << __func__ << ": key=" << key << dendl;
 
   operation::MetadataRemoveRequest<I> *request =
-    new operation::MetadataRemoveRequest<I>(m_image_ctx, on_finish, key);
+    new operation::MetadataRemoveRequest<I>(
+       m_image_ctx,
+       new C_NotifyUpdate<I>(m_image_ctx, on_finish), key);
   request->send();
 }
 

diff --git a/src/librbd/image/RefreshRequest.cc b/src/librbd/image/RefreshRequest.cc
index f8448dc0e015911d36269fa25c1a6551d788b217..4ec161698f5dc03e0ed6e6b5eb971c5f281f4251 100644 (file)
--- a/src/librbd/image/RefreshRequest.cc
+++ b/src/librbd/image/RefreshRequest.cc
@@ -1,6 +1,9 @@
 // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
 // vim: ts=8 sw=2 smarttab
 
+#include <boost/algorithm/string/predicate.hpp>
+#include "include/assert.h"
+
 #include "librbd/image/RefreshRequest.h"
 #include "common/dout.h"
 #include "common/errno.h"
@@ -22,6 +25,12 @@
 namespace librbd {
 namespace image {
 
+namespace {
+
+const uint64_t MAX_METADATA_ITEMS = 128;
+
+}
+
 using util::create_rados_callback;
 using util::create_async_context_callback;
 using util::create_context_callback;
@@ -285,6 +294,59 @@ Context *RefreshRequest<I>::handle_v2_get_mutable_metadata(int *result) {
     m_incomplete_update = true;
   }
 
+  send_v2_get_metadata();
+  return nullptr;
+}
+
+template <typename I>
+void RefreshRequest<I>::send_v2_get_metadata() {
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 10) << this << " " << __func__ << ": "
+                 << "start_key=" << m_last_metadata_key << dendl;
+
+  librados::ObjectReadOperation op;
+  cls_client::metadata_list_start(&op, m_last_metadata_key, MAX_METADATA_ITEMS);
+
+  using klass = RefreshRequest<I>;
+  librados::AioCompletion *comp =
+    create_rados_callback<klass, &klass::handle_v2_get_metadata>(this);
+  m_out_bl.clear();
+  m_image_ctx.md_ctx.aio_operate(m_image_ctx.header_oid, comp, &op,
+                                  &m_out_bl);
+  comp->release();
+}
+
+template <typename I>
+Context *RefreshRequest<I>::handle_v2_get_metadata(int *result) {
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 10) << this << " " << __func__ << ": r=" << *result << dendl;
+
+  std::map<std::string, bufferlist> metadata;
+  if (*result == 0) {
+    bufferlist::iterator it = m_out_bl.begin();
+    *result = cls_client::metadata_list_finish(&it, &metadata);
+  }
+
+  if (*result == -EOPNOTSUPP || *result == -EIO) {
+    ldout(cct, 10) << "config metadata not supported by OSD" << dendl;
+  } else if (*result < 0) {
+    lderr(cct) << "failed to retrieve metadata: " << cpp_strerror(*result)
+               << dendl;
+    return m_on_finish;
+  }
+
+  if (!metadata.empty()) {
+    m_metadata.insert(metadata.begin(), metadata.end());
+    m_last_metadata_key = metadata.rbegin()->first;
+    if (boost::starts_with(m_last_metadata_key,
+                           ImageCtx::METADATA_CONF_PREFIX)) {
+      send_v2_get_metadata();
+      return nullptr;
+    }
+  }
+
+  m_image_ctx.apply_metadata(m_metadata);
+
   send_v2_get_flags();
   return nullptr;
 }

diff --git a/src/librbd/image/RefreshRequest.h b/src/librbd/image/RefreshRequest.h
index 1bff99621ce5aaa4640eb45f9fb356ee47600683..33f4d2568af551236b9bc41c18442398208bd32a 100644 (file)
--- a/src/librbd/image/RefreshRequest.h
+++ b/src/librbd/image/RefreshRequest.h
@@ -52,6 +52,9 @@ private:
    *    \-----> V2_GET_MUTABLE_METADATA                    <apply>
    *                |                                         |
    *                v                                         |
+   *            V2_GET_METADATA                               |
+   *                |                                         |
+   *                v                                         |
    *            V2_GET_FLAGS                                  |
    *                |                                         |
    *                v                                         |
@@ -127,6 +130,10 @@ private:
   uint64_t m_features = 0;
   uint64_t m_incompatible_features = 0;
   uint64_t m_flags = 0;
+
+  std::string m_last_metadata_key;
+  std::map<std::string, bufferlist> m_metadata;
+
   std::string m_object_prefix;
   ParentInfo m_parent_md;
   cls::rbd::GroupSpec m_group_spec;
@@ -163,6 +170,9 @@ private:
   void send_v2_get_mutable_metadata();
   Context *handle_v2_get_mutable_metadata(int *result);
 
+  void send_v2_get_metadata();
+  Context *handle_v2_get_metadata(int *result);
+
   void send_v2_get_flags();
   Context *handle_v2_get_flags(int *result);
 

diff --git a/src/test/librbd/image/test_mock_RefreshRequest.cc b/src/test/librbd/image/test_mock_RefreshRequest.cc
index 5c50d8131a691e7dc0b20664d3d9fbef652a86d0..07490146a56b0a5f5c62fea42af6f9e2b8d297f6 100644 (file)
--- a/src/test/librbd/image/test_mock_RefreshRequest.cc
+++ b/src/test/librbd/image/test_mock_RefreshRequest.cc
@@ -155,6 +155,16 @@ public:
     }
   }
 
+  void expect_get_metadata(MockRefreshImageCtx &mock_image_ctx, int r) {
+    auto &expect = EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx),
+                               exec(mock_image_ctx.header_oid, _, StrEq("rbd"), StrEq("metadata_list"), _, _, _));
+    if (r < 0) {
+      expect.WillOnce(Return(r));
+    } else {
+      expect.WillOnce(DoDefault());
+    }
+  }
+
   void expect_get_flags(MockRefreshImageCtx &mock_image_ctx, int r) {
     auto &expect = EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx),
                                exec(mock_image_ctx.header_oid, _, StrEq("rbd"), StrEq("get_flags"), _, _, _));
@@ -216,6 +226,12 @@ public:
     }
   }
 
+  void expect_apply_metadata(MockRefreshImageCtx &mock_image_ctx,
+                            int r) {
+    EXPECT_CALL(mock_image_ctx, apply_metadata(_))
+                 .WillOnce(Return(r));
+  }
+
   void expect_add_snap(MockRefreshImageCtx &mock_image_ctx,
                        const std::string &snap_name, uint64_t snap_id) {
     EXPECT_CALL(mock_image_ctx, add_snap(_, snap_name, snap_id, _, _, _, _, _));
@@ -398,6 +414,8 @@ TEST_F(TestMockImageRefreshRequest, SuccessV2) {
 
   InSequence seq;
   expect_get_mutable_metadata(mock_image_ctx, 0);
+  expect_get_metadata(mock_image_ctx, 0);
+  expect_apply_metadata(mock_image_ctx, 0);
   expect_get_flags(mock_image_ctx, 0);
   expect_get_group(mock_image_ctx, 0);
   expect_refresh_parent_is_required(mock_refresh_parent_request, false);
@@ -427,6 +445,8 @@ TEST_F(TestMockImageRefreshRequest, SuccessSnapshotV2) {
 
   InSequence seq;
   expect_get_mutable_metadata(mock_image_ctx, 0);
+  expect_get_metadata(mock_image_ctx, 0);
+  expect_apply_metadata(mock_image_ctx, 0);
   expect_get_flags(mock_image_ctx, 0);
   expect_get_flags(mock_image_ctx, 0);
   expect_get_group(mock_image_ctx, 0);
@@ -462,6 +482,8 @@ TEST_F(TestMockImageRefreshRequest, SuccessSetSnapshotV2) {
 
   InSequence seq;
   expect_get_mutable_metadata(mock_image_ctx, 0);
+  expect_get_metadata(mock_image_ctx, 0);
+  expect_apply_metadata(mock_image_ctx, 0);
   expect_get_flags(mock_image_ctx, 0);
   expect_get_flags(mock_image_ctx, 0);
   expect_get_group(mock_image_ctx, 0);
@@ -516,6 +538,8 @@ TEST_F(TestMockImageRefreshRequest, SuccessChild) {
 
   InSequence seq;
   expect_get_mutable_metadata(mock_image_ctx, 0);
+  expect_get_metadata(mock_image_ctx, 0);
+  expect_apply_metadata(mock_image_ctx, 0);
   expect_get_flags(mock_image_ctx, 0);
   expect_get_group(mock_image_ctx, 0);
   expect_refresh_parent_is_required(*mock_refresh_parent_request, true);
@@ -566,6 +590,8 @@ TEST_F(TestMockImageRefreshRequest, SuccessChildDontOpenParent) {
 
   InSequence seq;
   expect_get_mutable_metadata(mock_image_ctx, 0);
+  expect_get_metadata(mock_image_ctx, 0);
+  expect_apply_metadata(mock_image_ctx, 0);
   expect_get_flags(mock_image_ctx, 0);
   expect_get_group(mock_image_ctx, 0);
   if (ictx->test_features(RBD_FEATURE_EXCLUSIVE_LOCK)) {
@@ -628,6 +654,8 @@ TEST_F(TestMockImageRefreshRequest, DisableExclusiveLock) {
   // and journaling were never enabled (or active)
   InSequence seq;
   expect_get_mutable_metadata(mock_image_ctx, 0);
+  expect_get_metadata(mock_image_ctx, 0);
+  expect_apply_metadata(mock_image_ctx, 0);
   expect_get_flags(mock_image_ctx, 0);
   expect_get_group(mock_image_ctx, 0);
   expect_refresh_parent_is_required(mock_refresh_parent_request, false);
@@ -679,6 +707,8 @@ TEST_F(TestMockImageRefreshRequest, DisableExclusiveLockWhileAcquiringLock) {
   // and journaling were never enabled (or active)
   InSequence seq;
   expect_get_mutable_metadata(mock_image_ctx, 0);
+  expect_get_metadata(mock_image_ctx, 0);
+  expect_apply_metadata(mock_image_ctx, 0);
   expect_get_flags(mock_image_ctx, 0);
   expect_get_group(mock_image_ctx, 0);
   expect_refresh_parent_is_required(mock_refresh_parent_request, false);
@@ -720,6 +750,8 @@ TEST_F(TestMockImageRefreshRequest, JournalDisabledByPolicy) {
 
   InSequence seq;
   expect_get_mutable_metadata(mock_image_ctx, 0);
+  expect_get_metadata(mock_image_ctx, 0);
+  expect_apply_metadata(mock_image_ctx, 0);
   expect_get_flags(mock_image_ctx, 0);
   expect_get_group(mock_image_ctx, 0);
   expect_refresh_parent_is_required(mock_refresh_parent_request, false);
@@ -766,6 +798,8 @@ TEST_F(TestMockImageRefreshRequest, EnableJournalWithExclusiveLock) {
   // journal should be immediately opened if exclusive lock owned
   InSequence seq;
   expect_get_mutable_metadata(mock_image_ctx, 0);
+  expect_get_metadata(mock_image_ctx, 0);
+  expect_apply_metadata(mock_image_ctx, 0);
   expect_get_flags(mock_image_ctx, 0);
   expect_get_group(mock_image_ctx, 0);
   expect_refresh_parent_is_required(mock_refresh_parent_request, false);
@@ -811,6 +845,8 @@ TEST_F(TestMockImageRefreshRequest, EnableJournalWithoutExclusiveLock) {
   // do not open the journal if exclusive lock is not owned
   InSequence seq;
   expect_get_mutable_metadata(mock_image_ctx, 0);
+  expect_get_metadata(mock_image_ctx, 0);
+  expect_apply_metadata(mock_image_ctx, 0);
   expect_get_flags(mock_image_ctx, 0);
   expect_get_group(mock_image_ctx, 0);
   expect_refresh_parent_is_required(mock_refresh_parent_request, false);
@@ -854,6 +890,8 @@ TEST_F(TestMockImageRefreshRequest, DisableJournal) {
   // verify journal is closed if feature disabled
   InSequence seq;
   expect_get_mutable_metadata(mock_image_ctx, 0);
+  expect_get_metadata(mock_image_ctx, 0);
+  expect_apply_metadata(mock_image_ctx, 0);
   expect_get_flags(mock_image_ctx, 0);
   expect_get_group(mock_image_ctx, 0);
   expect_refresh_parent_is_required(mock_refresh_parent_request, false);
@@ -897,6 +935,8 @@ TEST_F(TestMockImageRefreshRequest, EnableObjectMapWithExclusiveLock) {
   // object map should be immediately opened if exclusive lock owned
   InSequence seq;
   expect_get_mutable_metadata(mock_image_ctx, 0);
+  expect_get_metadata(mock_image_ctx, 0);
+  expect_apply_metadata(mock_image_ctx, 0);
   expect_get_flags(mock_image_ctx, 0);
   expect_get_group(mock_image_ctx, 0);
   expect_refresh_parent_is_required(mock_refresh_parent_request, false);
@@ -933,6 +973,8 @@ TEST_F(TestMockImageRefreshRequest, EnableObjectMapWithoutExclusiveLock) {
   // do not open the object map if exclusive lock is not owned
   InSequence seq;
   expect_get_mutable_metadata(mock_image_ctx, 0);
+  expect_get_metadata(mock_image_ctx, 0);
+  expect_apply_metadata(mock_image_ctx, 0);
   expect_get_flags(mock_image_ctx, 0);
   expect_get_group(mock_image_ctx, 0);
   expect_refresh_parent_is_required(mock_refresh_parent_request, false);
@@ -980,6 +1022,8 @@ TEST_F(TestMockImageRefreshRequest, DisableObjectMap) {
   // verify object map is closed if feature disabled
   InSequence seq;
   expect_get_mutable_metadata(mock_image_ctx, 0);
+  expect_get_metadata(mock_image_ctx, 0);
+  expect_apply_metadata(mock_image_ctx, 0);
   expect_get_flags(mock_image_ctx, 0);
   expect_get_group(mock_image_ctx, 0);
   expect_refresh_parent_is_required(mock_refresh_parent_request, false);
@@ -1018,6 +1062,8 @@ TEST_F(TestMockImageRefreshRequest, OpenObjectMapError) {
   // object map should be immediately opened if exclusive lock owned
   InSequence seq;
   expect_get_mutable_metadata(mock_image_ctx, 0);
+  expect_get_metadata(mock_image_ctx, 0);
+  expect_apply_metadata(mock_image_ctx, 0);
   expect_get_flags(mock_image_ctx, 0);
   expect_get_group(mock_image_ctx, 0);
   expect_refresh_parent_is_required(mock_refresh_parent_request, false);
@@ -1031,5 +1077,35 @@ TEST_F(TestMockImageRefreshRequest, OpenObjectMapError) {
   ASSERT_EQ(nullptr, mock_image_ctx.object_map);
 }
 
+TEST_F(TestMockImageRefreshRequest, ApplyMetadataError) {
+  REQUIRE_FORMAT_V2();
+
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  MockRefreshImageCtx mock_image_ctx(*ictx);
+  MockRefreshParentRequest mock_refresh_parent_request;
+  MockExclusiveLock mock_exclusive_lock;
+  expect_op_work_queue(mock_image_ctx);
+  expect_test_features(mock_image_ctx);
+
+  InSequence seq;
+  expect_get_mutable_metadata(mock_image_ctx, 0);
+  expect_get_metadata(mock_image_ctx, 0);
+  expect_apply_metadata(mock_image_ctx, -EINVAL);
+  expect_get_flags(mock_image_ctx, 0);
+  expect_get_group(mock_image_ctx, 0);
+  expect_refresh_parent_is_required(mock_refresh_parent_request, false);
+  if (ictx->test_features(RBD_FEATURE_EXCLUSIVE_LOCK)) {
+    expect_init_exclusive_lock(mock_image_ctx, mock_exclusive_lock, 0);
+  }
+
+  C_SaferCond ctx;
+  MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, false, false, &ctx);
+  req->send();
+
+  ASSERT_EQ(0, ctx.wait());
+}
+
 } // namespace image
 } // namespace librbd

diff --git a/src/test/librbd/mock/MockImageCtx.h b/src/test/librbd/mock/MockImageCtx.h
index e4f5930644e1dac63fc5b2481498142b3e9353d2..ca44657c8c2cb01c34862921d07f7da7d9d0b5c5 100644 (file)
--- a/src/test/librbd/mock/MockImageCtx.h
+++ b/src/test/librbd/mock/MockImageCtx.h
@@ -202,6 +202,7 @@ struct MockImageCtx {
                                          size_t, uint64_t, Context *, int, ZTracer::Trace *));
   MOCK_METHOD8(write_to_cache, void(object_t, const bufferlist&, size_t,
                                     uint64_t, Context *, int, uint64_t, ZTracer::Trace *));
+  MOCK_METHOD1(apply_metadata, int(const std::map<std::string, bufferlist> &));
 
   ImageCtx *image_ctx;
   CephContext *cct;