From: Jason Dillaman Date: Thu, 4 Feb 2016 18:46:50 +0000 (-0500) Subject: cls_journal: client registration should hold opaque data structure X-Git-Tag: v10.0.4~28^2~4 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=3f29e7b89a2e8ada0f2f229b590ead822e692dc8;p=ceph.git 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 --- diff --git a/src/cls/journal/cls_journal.cc b/src/cls/journal/cls_journal.cc index 08599a38bbf5..1e8d8da21a08 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 7f8af73af3cf..c72b6b93edba 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 acb4ae541027..a6d595d36d2a 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 437b380d9cf2..8a94d10530ad 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 &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 3e614c20b661..19a1fcbb56d1 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 33221f94ffd4..629bddc60f03 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 f87ae2424a44..32d58bb421ae 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 171021fe24e2..eafa5a8ae8b0 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 42e5b0e41eeb..09127b79e549 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 d55a527888a1..0b0e453272ca 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 std::ostream &operator<<(std::ostream &os, const typename Journal::State &state) { @@ -125,7 +119,8 @@ int Journal::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::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 c10ed3891ea5..bde728ffe91e 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 clients; ASSERT_EQ(0, client::client_list(ioctx, oid, &clients)); - std::set expected_clients = {Client("id1", "desc1")}; + std::set 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 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 expected_all_tags; std::set expected_filtered_tags; diff --git a/src/test/journal/RadosTestFixture.cc b/src/test/journal/RadosTestFixture.cc index d5f1b3262b6a..c965c522b61d 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 5a19910e564a..f7231fa2c238 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 d651d6f07bbe..28598c0b0ffd 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 9c99129502e0..df842d3aedfb 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 7b13fe3be384..d15e6543ee71 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;