cls_journal: client registration should hold opaque data structure

The opaque structure will support journal client applications to
store and retrieve complex state.

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

diff --git a/src/cls/journal/cls_journal.cc b/src/cls/journal/cls_journal.cc
index 08599a38bbf563d7f790b806b723124a0399ed74..1e8d8da21a080456feefd6be4790d3a20ede3500 100644 (file)
--- a/src/cls/journal/cls_journal.cc
+++ b/src/cls/journal/cls_journal.cc
@@ -496,7 +496,7 @@ int journal_set_active_set(cls_method_context_t hctx, bufferlist *in,
 /**
  * Input:
  * @param id (string) - unique client id
- * @param description (string) - human-readable description of the client
+ * @param data (bufferlist) - opaque data associated to client
  *
  * Output:
  * @returns 0 on success, negative error code on failure
@@ -504,11 +504,11 @@ int journal_set_active_set(cls_method_context_t hctx, bufferlist *in,
 int journal_client_register(cls_method_context_t hctx, bufferlist *in,
                             bufferlist *out) {
   std::string id;
-  std::string description;
+  bufferlist data;
   try {
     bufferlist::iterator iter = in->begin();
     ::decode(id, iter);
-    ::decode(description, iter);
+    ::decode(data, iter);
   } catch (const buffer::error &err) {
     CLS_ERR("failed to decode input parameters: %s", err.what());
     return -EINVAL;
@@ -522,7 +522,7 @@ int journal_client_register(cls_method_context_t hctx, bufferlist *in,
     return -EEXIST;
   }
 
-  cls::journal::Client client(id, description);
+  cls::journal::Client client(id, data);
   key = key_from_client_id(id);
   r = write_key(hctx, key, client);
   if (r < 0) {

diff --git a/src/cls/journal/cls_journal_client.cc b/src/cls/journal/cls_journal_client.cc
index 7f8af73af3cff809889c47ecb4dc1a8f9565e70a..c72b6b93edbaa73d4618ad10d4d79f31f1572829 100644 (file)
--- a/src/cls/journal/cls_journal_client.cc
+++ b/src/cls/journal/cls_journal_client.cc
@@ -224,17 +224,17 @@ void set_active_set(librados::ObjectWriteOperation *op, uint64_t object_set) {
 }
 
 void client_register(librados::ObjectWriteOperation *op,
-                     const std::string &id, const std::string &description) {
+                     const std::string &id, const bufferlist &data) {
   bufferlist bl;
   ::encode(id, bl);
-  ::encode(description, bl);
+  ::encode(data, bl);
   op->exec("journal", "client_register", bl);
 }
 
 int client_register(librados::IoCtx &ioctx, const std::string &oid,
-                    const std::string &id, const std::string &description) {
+                    const std::string &id, const bufferlist &data) {
   librados::ObjectWriteOperation op;
-  client_register(&op, id, description);
+  client_register(&op, id, data);
   return ioctx.operate(oid, &op);
 }
 

diff --git a/src/cls/journal/cls_journal_client.h b/src/cls/journal/cls_journal_client.h
index acb4ae5410274b1d6dcc8d32508bce608427f76a..a6d595d36d2aa88528ff96016a0915f44f504cc2 100644 (file)
--- a/src/cls/journal/cls_journal_client.h
+++ b/src/cls/journal/cls_journal_client.h
@@ -34,9 +34,9 @@ void set_active_set(librados::ObjectWriteOperation *op, uint64_t object_set);
 
 // journal client helpers
 void client_register(librados::ObjectWriteOperation *op,
-                     const std::string &id, const std::string &description);
+                     const std::string &id, const bufferlist &data);
 int client_register(librados::IoCtx &ioctx, const std::string &oid,
-                    const std::string &id, const std::string &description);
+                    const std::string &id, const bufferlist &data);
 int client_unregister(librados::IoCtx &ioctx, const std::string &oid,
                       const std::string &id);
 void client_commit(librados::ObjectWriteOperation *op, const std::string &id,

diff --git a/src/cls/journal/cls_journal_types.cc b/src/cls/journal/cls_journal_types.cc
index 437b380d9cf2821e4c632d746df48943f85b6d4e..8a94d10530ad292fe388f9a98a73a8648f07e9af 100644 (file)
--- a/src/cls/journal/cls_journal_types.cc
+++ b/src/cls/journal/cls_journal_types.cc
@@ -66,7 +66,7 @@ void ObjectSetPosition::generate_test_instances(
 void Client::encode(bufferlist& bl) const {
   ENCODE_START(1, 1, bl);
   ::encode(id, bl);
-  ::encode(description, bl);
+  ::encode(data, bl);
   ::encode(commit_position, bl);
   ENCODE_FINISH(bl);
 }
@@ -74,14 +74,17 @@ void Client::encode(bufferlist& bl) const {
 void Client::decode(bufferlist::iterator& iter) {
   DECODE_START(1, iter);
   ::decode(id, iter);
-  ::decode(description, iter);
+  ::decode(data, iter);
   ::decode(commit_position, iter);
   DECODE_FINISH(iter);
 }
 
 void Client::dump(Formatter *f) const {
   f->dump_string("id", id);
-  f->dump_string("description", description);
+
+  std::stringstream data_ss;
+  data.hexdump(data_ss);
+  f->dump_string("data", data_ss.str());
 
   f->open_object_section("commit_position");
   commit_position.dump(f);
@@ -89,9 +92,12 @@ void Client::dump(Formatter *f) const {
 }
 
 void Client::generate_test_instances(std::list<Client *> &o) {
+  bufferlist data;
+  data.append(std::string('1', 128));
+
   o.push_back(new Client());
-  o.push_back(new Client("id", "desc"));
-  o.push_back(new Client("id", "desc", {1, {{1, 120}, {2, 121}}}));
+  o.push_back(new Client("id", data));
+  o.push_back(new Client("id", data, {1, {{1, 120}, {2, 121}}}));
 }
 
 void Tag::encode(bufferlist& bl) const {
@@ -149,7 +155,9 @@ std::ostream &operator<<(std::ostream &os,
 
 std::ostream &operator<<(std::ostream &os, const Client &client) {
   os << "[id=" << client.id << ", "
-     << "description=" << client.description << ", "
+     << "data=";
+  client.data.hexdump(os);
+  os << ", "
      << "commit_position=" << client.commit_position << "]";
   return os;
 }

diff --git a/src/cls/journal/cls_journal_types.h b/src/cls/journal/cls_journal_types.h
index 3e614c20b661cfad536f91d4c0c427298552d8a6..19a1fcbb56d1bd0e000e1c016c65a60d95a61dc5 100644 (file)
--- a/src/cls/journal/cls_journal_types.h
+++ b/src/cls/journal/cls_journal_types.h
@@ -70,17 +70,17 @@ struct ObjectSetPosition {
 
 struct Client {
   std::string id;
-  std::string description;
+  bufferlist data;
   ObjectSetPosition commit_position;
 
   Client() {}
-  Client(const std::string& _id, const std::string& _description,
+  Client(const std::string& _id, const bufferlist &_data,
          const ObjectSetPosition &_commit_position = ObjectSetPosition())
-    : id(_id), description(_description), commit_position(_commit_position) {}
+    : id(_id), data(_data), commit_position(_commit_position) {}
 
   inline bool operator==(const Client &rhs) const {
     return (id == rhs.id &&
-            description == rhs.description &&
+            data.contents_equal(rhs.data) &&
             commit_position == rhs.commit_position);
   }
   inline bool operator<(const Client &rhs) const {

diff --git a/src/journal/JournalMetadata.cc b/src/journal/JournalMetadata.cc
index 33221f94ffd4ddcc66ab5a31678631efa309022d..629bddc60f031feb87e72387c13de4220c23298a 100644 (file)
--- a/src/journal/JournalMetadata.cc
+++ b/src/journal/JournalMetadata.cc
@@ -133,9 +133,9 @@ void JournalMetadata::shutdown() {
   m_ioctx.aio_flush();
 }
 
-int JournalMetadata::register_client(const std::string &description) {
+int JournalMetadata::register_client(const bufferlist &data) {
   ldout(m_cct, 10) << __func__ << ": " << m_client_id << dendl;
-  int r = client::client_register(m_ioctx, m_oid, m_client_id, description);
+  int r = client::client_register(m_ioctx, m_oid, m_client_id, data);
   if (r < 0) {
     lderr(m_cct) << "failed to register journal client '" << m_client_id
                  << "': " << cpp_strerror(r) << dendl;
@@ -315,7 +315,7 @@ void JournalMetadata::handle_refresh_complete(C_Refresh *refresh, int r) {
   if (r == 0) {
     Mutex::Locker locker(m_lock);
 
-    Client client(m_client_id, "");
+    Client client(m_client_id, bufferlist());
     RegisteredClients::iterator it = refresh->registered_clients.find(client);
     if (it != refresh->registered_clients.end()) {
       m_minimum_set = refresh->minimum_set;

diff --git a/src/journal/JournalMetadata.h b/src/journal/JournalMetadata.h
index f87ae2424a44ad8925158c4b59b5bca2961246c8..32d58bb421ae08f343b88fea385c31f3706768e8 100644 (file)
--- a/src/journal/JournalMetadata.h
+++ b/src/journal/JournalMetadata.h
@@ -51,7 +51,7 @@ public:
   void add_listener(Listener *listener);
   void remove_listener(Listener *listener);
 
-  int register_client(const std::string &description);
+  int register_client(const bufferlist &data);
   int unregister_client();
 
   inline const std::string &get_client_id() const {

diff --git a/src/journal/Journaler.cc b/src/journal/Journaler.cc
index 171021fe24e268dce2947e4aea19fdd8a6e9f4c2..eafa5a8ae8b0436092d45b7c4f16807329791d8e 100644 (file)
--- a/src/journal/Journaler.cc
+++ b/src/journal/Journaler.cc
@@ -159,8 +159,8 @@ int Journaler::remove(bool force) {
   return 0;
 }
 
-int Journaler::register_client(const std::string &description) {
-  return m_metadata->register_client(description);
+int Journaler::register_client(const bufferlist &data) {
+  return m_metadata->register_client(data);
 }
 
 int Journaler::unregister_client() {

diff --git a/src/journal/Journaler.h b/src/journal/Journaler.h
index 42e5b0e41eebc2e71a030ce7311e90891008aec1..09127b79e54936f1d3c93d779a692ecb517e6933 100644 (file)
--- a/src/journal/Journaler.h
+++ b/src/journal/Journaler.h
@@ -42,7 +42,7 @@ public:
   void init(Context *on_init);
   void shutdown();
 
-  int register_client(const std::string &description);
+  int register_client(const bufferlist &data);
   int unregister_client();
 
   void start_replay(ReplayHandler *replay_handler);

diff --git a/src/librbd/Journal.cc b/src/librbd/Journal.cc
index d55a527888a1919133e5a939a4bceb9033cbbbbf..0b0e453272cad69b78c9a804552b75d8c1d5c81c 100644 (file)
--- a/src/librbd/Journal.cc
+++ b/src/librbd/Journal.cc
@@ -23,12 +23,6 @@ namespace librbd {
 using util::create_async_context_callback;
 using util::create_context_callback;
 
-namespace {
-
-const std::string CLIENT_DESCRIPTION = "master image";
-
-} // anonymous namespace
-
 template <typename I>
 std::ostream &operator<<(std::ostream &os,
                          const typename Journal<I>::State &state) {
@@ -125,7 +119,8 @@ int Journal<I>::create(librados::IoCtx &io_ctx, const std::string &image_id,
     return r;
   }
 
-  r = journaler.register_client(CLIENT_DESCRIPTION);
+  // TODO register with librbd payload
+  r = journaler.register_client(bufferlist());
   if (r < 0) {
     lderr(cct) << "failed to register client: " << cpp_strerror(r) << dendl;
     return r;
@@ -200,7 +195,7 @@ int Journal<I>::reset(librados::IoCtx &io_ctx, const std::string &image_id) {
     lderr(cct) << "failed to create journal: " << cpp_strerror(r) << dendl;
     return r;
   }
-  r = journaler.register_client(CLIENT_DESCRIPTION);
+  r = journaler.register_client(bufferlist());
   if (r < 0) {
     lderr(cct) << "failed to register client: " << cpp_strerror(r) << dendl;
     return r;

diff --git a/src/test/cls_journal/test_cls_journal.cc b/src/test/cls_journal/test_cls_journal.cc
index c10ed3891ea570697a5e74b6e322423018b9373d..bde728ffe91e277fc88f3fc62fed64a047c6070e 100644 (file)
--- a/src/test/cls_journal/test_cls_journal.cc
+++ b/src/test/cls_journal/test_cls_journal.cc
@@ -198,12 +198,12 @@ TEST_F(TestClsJournal, ClientRegister) {
 
   std::string oid = get_temp_image_name();
 
-  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", "desc1"));
+  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", bufferlist()));
 
   std::set<Client> clients;
   ASSERT_EQ(0, client::client_list(ioctx, oid, &clients));
 
-  std::set<Client> expected_clients = {Client("id1", "desc1")};
+  std::set<Client> expected_clients = {Client("id1", bufferlist())};
   ASSERT_EQ(expected_clients, clients);
 }
 
@@ -213,8 +213,8 @@ TEST_F(TestClsJournal, ClientRegisterDuplicate) {
 
   std::string oid = get_temp_image_name();
 
-  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", "desc1"));
-  ASSERT_EQ(-EEXIST, client::client_register(ioctx, oid, "id1", "desc2"));
+  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", bufferlist()));
+  ASSERT_EQ(-EEXIST, client::client_register(ioctx, oid, "id1", bufferlist()));
 }
 
 TEST_F(TestClsJournal, ClientUnregister) {
@@ -223,7 +223,7 @@ TEST_F(TestClsJournal, ClientUnregister) {
 
   std::string oid = get_temp_image_name();
 
-  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", "desc1"));
+  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", bufferlist()));
   ASSERT_EQ(0, client::client_unregister(ioctx, oid, "id1"));
 }
 
@@ -233,7 +233,7 @@ TEST_F(TestClsJournal, ClientUnregisterDNE) {
 
   std::string oid = get_temp_image_name();
 
-  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", "desc1"));
+  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", bufferlist()));
   ASSERT_EQ(0, client::client_unregister(ioctx, oid, "id1"));
   ASSERT_EQ(-ENOENT, client::client_unregister(ioctx, oid, "id1"));
 }
@@ -245,8 +245,8 @@ TEST_F(TestClsJournal, ClientUnregisterPruneTags) {
   std::string oid = get_temp_image_name();
 
   ASSERT_EQ(0, client::create(ioctx, oid, 2, 2, ioctx.get_id()));
-  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", "desc1"));
-  ASSERT_EQ(0, client::client_register(ioctx, oid, "id2", "desc2"));
+  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", bufferlist()));
+  ASSERT_EQ(0, client::client_register(ioctx, oid, "id2", bufferlist()));
 
   ASSERT_EQ(0, client::tag_create(ioctx, oid, 0, Tag::TAG_CLASS_NEW,
                                   bufferlist()));
@@ -274,7 +274,7 @@ TEST_F(TestClsJournal, ClientCommit) {
   std::string oid = get_temp_image_name();
 
   ASSERT_EQ(0, client::create(ioctx, oid, 2, 2, ioctx.get_id()));
-  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", "desc1"));
+  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", bufferlist()));
 
   cls::journal::EntryPositions entry_positions;
   entry_positions = {
@@ -291,7 +291,7 @@ TEST_F(TestClsJournal, ClientCommit) {
   ASSERT_EQ(0, client::client_list(ioctx, oid, &clients));
 
   std::set<Client> expected_clients = {
-    Client("id1", "desc1", object_set_position)};
+    Client("id1", bufferlist(), object_set_position)};
   ASSERT_EQ(expected_clients, clients);
 }
 
@@ -302,7 +302,7 @@ TEST_F(TestClsJournal, ClientCommitInvalid) {
   std::string oid = get_temp_image_name();
 
   ASSERT_EQ(0, client::create(ioctx, oid, 2, 2, ioctx.get_id()));
-  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", "desc1"));
+  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", bufferlist()));
 
   cls::journal::EntryPositions entry_positions;
   entry_positions = {
@@ -342,8 +342,8 @@ TEST_F(TestClsJournal, ClientList) {
   librados::ObjectWriteOperation op1;
   for (uint32_t i = 0; i < 512; ++i) {
     std::string id =  "id" + stringify(i + 1);
-    expected_clients.insert(Client(id, ""));
-    client::client_register(&op1, id, "");
+    expected_clients.insert(Client(id, bufferlist()));
+    client::client_register(&op1, id, bufferlist());
   }
   ASSERT_EQ(0, ioctx.operate(oid, &op1));
 
@@ -371,7 +371,7 @@ TEST_F(TestClsJournal, GetNextTagTid) {
   ASSERT_EQ(-ENOENT, client::get_next_tag_tid(ioctx, oid, &tag_tid));
 
   ASSERT_EQ(0, client::create(ioctx, oid, 2, 2, ioctx.get_id()));
-  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", "desc1"));
+  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", bufferlist()));
 
   ASSERT_EQ(0, client::get_next_tag_tid(ioctx, oid, &tag_tid));
   ASSERT_EQ(0U, tag_tid);
@@ -392,7 +392,7 @@ TEST_F(TestClsJournal, TagCreate) {
                                         bufferlist()));
 
   ASSERT_EQ(0, client::create(ioctx, oid, 2, 2, ioctx.get_id()));
-  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", "desc1"));
+  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", bufferlist()));
 
   ASSERT_EQ(-ESTALE, client::tag_create(ioctx, oid, 1, Tag::TAG_CLASS_NEW,
                                         bufferlist()));
@@ -421,7 +421,7 @@ TEST_F(TestClsJournal, TagCreatePrunesTags) {
   std::string oid = get_temp_image_name();
 
   ASSERT_EQ(0, client::create(ioctx, oid, 2, 2, ioctx.get_id()));
-  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", "desc1"));
+  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", bufferlist()));
 
   ASSERT_EQ(0, client::tag_create(ioctx, oid, 0, Tag::TAG_CLASS_NEW,
                                   bufferlist()));
@@ -450,7 +450,7 @@ TEST_F(TestClsJournal, TagList) {
   std::string oid = get_temp_image_name();
 
   ASSERT_EQ(0, client::create(ioctx, oid, 2, 2, ioctx.get_id()));
-  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", "desc1"));
+  ASSERT_EQ(0, client::client_register(ioctx, oid, "id1", bufferlist()));
 
   std::set<Tag> expected_all_tags;
   std::set<Tag> expected_filtered_tags;

diff --git a/src/test/journal/RadosTestFixture.cc b/src/test/journal/RadosTestFixture.cc
index d5f1b3262b6adbf6e7f96216a56fbeba9a49078c..c965c522b61d160776e6e21377034daddb3ec209 100644 (file)
--- a/src/test/journal/RadosTestFixture.cc
+++ b/src/test/journal/RadosTestFixture.cc
@@ -52,7 +52,9 @@ int RadosTestFixture::append(const std::string &oid, const bufferlist &bl) {
 int RadosTestFixture::client_register(const std::string &oid,
                                       const std::string &id,
                                       const std::string &description) {
-  return cls::journal::client::client_register(m_ioctx, oid, id, description);
+  bufferlist data;
+  data.append(description);
+  return cls::journal::client::client_register(m_ioctx, oid, id, data);
 }
 
 int RadosTestFixture::client_commit(const std::string &oid,

diff --git a/src/test/journal/test_Journaler.cc b/src/test/journal/test_Journaler.cc
index 5a19910e564a1512f24f596b999f7fcba0e798f0..f7231fa2c238ff5f26af3ccc5245f7ca38accf00 100644 (file)
--- a/src/test/journal/test_Journaler.cc
+++ b/src/test/journal/test_Journaler.cc
@@ -40,7 +40,9 @@ public:
 
   int register_client(const std::string &client_id, const std::string &desc) {
     journal::Journaler journaler(m_ioctx, m_journal_id, client_id, 5);
-    return journaler.register_client(desc);
+    bufferlist data;
+    data.append(desc);
+    return journaler.register_client(data);
   }
 
   static uint64_t _journal_id;

diff --git a/src/test/librbd/journal/test_Entries.cc b/src/test/librbd/journal/test_Entries.cc
index d651d6f07bbe4dcf5f128eea2676cea048002083..28598c0b0ffd871115b57459af18398d4468bce3 100644 (file)
--- a/src/test/librbd/journal/test_Entries.cc
+++ b/src/test/librbd/journal/test_Entries.cc
@@ -67,7 +67,7 @@ public:
     journal::Journaler *journaler = new journal::Journaler(
       ictx->md_ctx, ictx->id, "dummy client", 1);
 
-    int r = journaler->register_client("unit test client");
+    int r = journaler->register_client(bufferlist());
     if (r < 0) {
       ADD_FAILURE() << "failed to register journal client";
       delete journaler;

diff --git a/src/test/librbd/test_mock_Journal.cc b/src/test/librbd/test_mock_Journal.cc
index 9c99129502e0fa57d73ecd2c603138e6e0854e0d..df842d3aedfbb74e8cf272f120d980a8ce857528 100644 (file)
--- a/src/test/librbd/test_mock_Journal.cc
+++ b/src/test/librbd/test_mock_Journal.cc
@@ -116,7 +116,7 @@ struct MockJournalerProxy {
   int remove(bool force) {
     return -EINVAL;
   }
-  int register_client(const std::string &description) {
+  int register_client(const bufferlist &data) {
     return -EINVAL;
   }
 

diff --git a/src/tools/rbd/action/Journal.cc b/src/tools/rbd/action/Journal.cc
index 7b13fe3be384f9f5148f52de034cd1016ffeb611..d15e6543ee7188c5c95e618cf92b374eb060a002 100644 (file)
--- a/src/tools/rbd/action/Journal.cc
+++ b/src/tools/rbd/action/Journal.cc
@@ -164,10 +164,8 @@ static int do_reset_journal(librados::IoCtx& io_ctx,
     return r;
   }
 
-  // XXXMG
-  const std::string CLIENT_DESCRIPTION = "master image";
-
-  r = journaler.register_client(CLIENT_DESCRIPTION);
+  // TODO register with librbd payload
+  r = journaler.register_client(bufferlist());
   if (r < 0) {
     std::cerr << "failed to register client: " << cpp_strerror(r) << std::endl;
     return r;
@@ -185,7 +183,8 @@ public:
   int init() {
     int r;
 
-    r = register_client("rbd journal");
+    // TODO register with librbd payload
+    r = register_client(bufferlist());
     if (r < 0) {
       std::cerr << "failed to register client: " << cpp_strerror(r)
                << std::endl;