From: Mykola Golub Date: Mon, 20 Jul 2020 12:14:47 +0000 (+0100) Subject: ceph-immutable-object-cache: don't return empty path for older clients X-Git-Tag: v15.2.9~122^2~92^2~2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=a1b9620fea8d33432203e6aa143441c215e8a244;p=ceph.git ceph-immutable-object-cache: don't return empty path for older clients They would crash. Now they will try to read a non-existing file. Signed-off-by: Mykola Golub (cherry picked from commit 955a185f410152c87c52377153c397e845edbf1d) --- diff --git a/src/test/immutable_object_cache/test_object_store.cc b/src/test/immutable_object_cache/test_object_store.cc index 5e044322ee01..736928fe0e7d 100644 --- a/src/test/immutable_object_cache/test_object_store.cc +++ b/src/test/immutable_object_cache/test_object_store.cc @@ -68,7 +68,8 @@ public: void lookup_object_cache_store(std::string pool_name, std::string vol_name, std::string obj_name, int& ret) { std::string cache_path; - ret = m_object_cache_store->lookup_object(pool_name, 1, 2, obj_name, cache_path); + ret = m_object_cache_store->lookup_object(pool_name, 1, 2, obj_name, true, + cache_path); } void TearDown() override { diff --git a/src/tools/immutable_object_cache/CacheClient.cc b/src/tools/immutable_object_cache/CacheClient.cc index 60ba9f52fc12..eeec92202ad7 100644 --- a/src/tools/immutable_object_cache/CacheClient.cc +++ b/src/tools/immutable_object_cache/CacheClient.cc @@ -3,6 +3,7 @@ #include "CacheClient.h" #include "common/Cond.h" +#include "common/version.h" #define dout_context g_ceph_context #define dout_subsys ceph_subsys_immutable_obj_cache @@ -376,7 +377,8 @@ namespace immutable_obj_cache { // TODO : re-implement this method int CacheClient::register_client(Context* on_finish) { ObjectCacheRequest* reg_req = new ObjectCacheRegData(RBDSC_REGISTER, - m_sequence_id++); + m_sequence_id++, + ceph_version_to_str()); reg_req->encode(); bufferlist bl; diff --git a/src/tools/immutable_object_cache/CacheController.cc b/src/tools/immutable_object_cache/CacheController.cc index c033ca49b611..1fade8f84b4b 100644 --- a/src/tools/immutable_object_cache/CacheController.cc +++ b/src/tools/immutable_object_cache/CacheController.cc @@ -102,6 +102,9 @@ void CacheController::handle_request(CacheSession* session, case RBDSC_REGISTER: { // TODO(dehao): skip register and allow clients to lookup directly + auto req_reg_data = reinterpret_cast (req); + session->set_client_version(req_reg_data->version); + ObjectCacheRequest* reply = new ObjectCacheRegReplyData( RBDSC_REGISTER_REPLY, req->seq); session->send(reply); @@ -112,9 +115,11 @@ void CacheController::handle_request(CacheSession* session, std::string cache_path; ObjectCacheReadData* req_read_data = reinterpret_cast (req); + bool return_dne_path = session->client_version().empty(); int ret = m_object_cache_store->lookup_object( req_read_data->pool_namespace, req_read_data->pool_id, - req_read_data->snap_id, req_read_data->oid, cache_path); + req_read_data->snap_id, req_read_data->oid, return_dne_path, + cache_path); ObjectCacheRequest* reply = nullptr; if (ret != OBJ_CACHE_PROMOTED && ret != OBJ_CACHE_DNE) { reply = new ObjectCacheReadRadosData(RBDSC_READ_RADOS, req->seq); diff --git a/src/tools/immutable_object_cache/CacheSession.cc b/src/tools/immutable_object_cache/CacheSession.cc index 5bf730354ab3..d00eac4f4865 100644 --- a/src/tools/immutable_object_cache/CacheSession.cc +++ b/src/tools/immutable_object_cache/CacheSession.cc @@ -31,6 +31,14 @@ stream_protocol::socket& CacheSession::socket() { return m_dm_socket; } +void CacheSession::set_client_version(const std::string &version) { + m_client_version = version; +} + +const std::string &CacheSession::client_version() const { + return m_client_version; +} + void CacheSession::close() { if (m_dm_socket.is_open()) { boost::system::error_code close_ec; diff --git a/src/tools/immutable_object_cache/CacheSession.h b/src/tools/immutable_object_cache/CacheSession.h index d530b529c8c2..b2890992f1a0 100644 --- a/src/tools/immutable_object_cache/CacheSession.h +++ b/src/tools/immutable_object_cache/CacheSession.h @@ -36,11 +36,16 @@ class CacheSession : public std::enable_shared_from_this { void fault(const boost::system::error_code& ec); void send(ObjectCacheRequest* msg); + void set_client_version(const std::string &version); + const std::string &client_version() const; + private: stream_protocol::socket m_dm_socket; ProcessMsg m_server_process_msg; CephContext* m_cct; + std::string m_client_version; + bufferptr m_bp_header; }; diff --git a/src/tools/immutable_object_cache/ObjectCacheStore.cc b/src/tools/immutable_object_cache/ObjectCacheStore.cc index aec72f3aa50a..a0b2b27ce162 100644 --- a/src/tools/immutable_object_cache/ObjectCacheStore.cc +++ b/src/tools/immutable_object_cache/ObjectCacheStore.cc @@ -181,6 +181,7 @@ int ObjectCacheStore::handle_promote_callback(int ret, bufferlist* read_buf, int ObjectCacheStore::lookup_object(std::string pool_nspace, uint64_t pool_id, uint64_t snap_id, std::string object_name, + bool return_dne_path, std::string& target_cache_file_path) { ldout(m_cct, 20) << "object name = " << object_name << " in pool ID : " << pool_id << dendl; @@ -202,6 +203,10 @@ int ObjectCacheStore::lookup_object(std::string pool_nspace, target_cache_file_path = get_cache_file_path(cache_file_name); return ret; case OBJ_CACHE_DNE: + if (return_dne_path) { + target_cache_file_path = get_cache_file_path(cache_file_name); + } + return ret; case OBJ_CACHE_SKIP: return ret; default: diff --git a/src/tools/immutable_object_cache/ObjectCacheStore.h b/src/tools/immutable_object_cache/ObjectCacheStore.h index eba003412e0c..270a93be4522 100644 --- a/src/tools/immutable_object_cache/ObjectCacheStore.h +++ b/src/tools/immutable_object_cache/ObjectCacheStore.h @@ -31,6 +31,7 @@ class ObjectCacheStore { int lookup_object(std::string pool_nspace, uint64_t pool_id, uint64_t snap_id, std::string object_name, + bool return_dne_path, std::string& target_cache_file_path); private: diff --git a/src/tools/immutable_object_cache/Types.cc b/src/tools/immutable_object_cache/Types.cc index a0a4c6352aa5..e65e94bee1ed 100644 --- a/src/tools/immutable_object_cache/Types.cc +++ b/src/tools/immutable_object_cache/Types.cc @@ -40,12 +40,24 @@ void ObjectCacheRequest::decode(bufferlist& bl) { ObjectCacheRegData::ObjectCacheRegData() {} ObjectCacheRegData::ObjectCacheRegData(uint16_t t, uint64_t s) : ObjectCacheRequest(t, s) {} +ObjectCacheRegData::ObjectCacheRegData(uint16_t t, uint64_t s, + const std::string &version) + : ObjectCacheRequest(t, s), + version(version) { +} ObjectCacheRegData::~ObjectCacheRegData() {} -void ObjectCacheRegData::encode_payload() {} +void ObjectCacheRegData::encode_payload() { + ceph::encode(version, payload); +} -void ObjectCacheRegData::decode_payload(bufferlist::const_iterator i) {} +void ObjectCacheRegData::decode_payload(bufferlist::const_iterator i) { + if (i.end()) { + return; + } + ceph::decode(version, i); +} ObjectCacheRegReplyData::ObjectCacheRegReplyData() {} ObjectCacheRegReplyData::ObjectCacheRegReplyData(uint16_t t, uint64_t s) diff --git a/src/tools/immutable_object_cache/Types.h b/src/tools/immutable_object_cache/Types.h index 7967d16564f0..5fab1ec4897b 100644 --- a/src/tools/immutable_object_cache/Types.h +++ b/src/tools/immutable_object_cache/Types.h @@ -57,13 +57,15 @@ class ObjectCacheRequest { class ObjectCacheRegData : public ObjectCacheRequest { public: + std::string version; ObjectCacheRegData(); + ObjectCacheRegData(uint16_t t, uint64_t s, const std::string &version); ObjectCacheRegData(uint16_t t, uint64_t s); ~ObjectCacheRegData() override; void encode_payload() override; void decode_payload(bufferlist::const_iterator bl) override; uint16_t get_request_type() override { return RBDSC_REGISTER; } - bool payload_empty() override { return true; } + bool payload_empty() override { return false; } }; class ObjectCacheRegReplyData : public ObjectCacheRequest {