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>

16 files changed, 67 insertions, 61 deletions

diff --git a/src/cls/journal/cls_journal.cc b/src/cls/journal/cls_journal.cc
index 08599a38bbf..1e8d8da21a0 100644
--- 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 7f8af73af3c..c72b6b93edb 100644
--- 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 acb4ae54102..a6d595d36d2 100644
--- 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 437b380d9cf..8a94d10530a 100644
--- 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 3e614c20b66..19a1fcbb56d 100644
--- 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 33221f94ffd..629bddc60f0 100644
--- 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 f87ae2424a4..32d58bb421a 100644
--- 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 171021fe24e..eafa5a8ae8b 100644
--- 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 42e5b0e41ee..09127b79e54 100644
--- 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 d55a527888a..0b0e453272c 100644
--- 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 c10ed3891ea..bde728ffe91 100644
--- 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 d5f1b3262b6..c965c522b61 100644
--- 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 5a19910e564..f7231fa2c23 100644
--- 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 d651d6f07bb..28598c0b0ff 100644
--- 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 9c99129502e..df842d3aedf 100644
--- 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 7b13fe3be38..d15e6543ee7 100644
--- 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;