From: Daniel Gryniewicz Date: Fri, 11 Apr 2025 16:11:34 +0000 (-0400) Subject: RGW - zipper - Remove target from get_obj_attrs() X-Git-Tag: v21.0.0~209^2~52^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=f30a6a8f726870d6c11d7de6c81197ed52e35d08;p=ceph.git RGW - zipper - Remove target from get_obj_attrs() The target argument to get_obj_attrs() was only used internally to RadosStore and DBStore, and never by callers of the API. Remove it entirely, to reduce API complexity. Signed-off-by: Daniel Gryniewicz --- diff --git a/src/rgw/driver/d4n/rgw_sal_d4n.cc b/src/rgw/driver/d4n/rgw_sal_d4n.cc index 257803c6d3cf..d1971413fb8b 100644 --- a/src/rgw/driver/d4n/rgw_sal_d4n.cc +++ b/src/rgw/driver/d4n/rgw_sal_d4n.cc @@ -1456,8 +1456,7 @@ bool D4NFilterObject::check_head_exists_in_cache_get_oid(const DoutPrefixProvide return found_in_cache; } -int D4NFilterObject::get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp, - rgw_obj* target_obj) +int D4NFilterObject::get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp) { bool is_latest_version = true; if (this->have_instance()) { @@ -1476,17 +1475,14 @@ int D4NFilterObject::get_obj_attrs(optional_yield y, const DoutPrefixProvider* d rgw::sal::Attrs attrs; std::string version; ldpp_dout(dpp, 10) << "D4NFilterObject::" << __func__ << "(): Fetching attrs from backend store." << dendl; - auto ret = next->get_obj_attrs(y, dpp, target_obj); - if (ret < 0 || !target_obj) { - if (!target_obj) { - ret = -ENOENT; - } + auto ret = next->get_obj_attrs(y, dpp); + if (ret < 0) { ldpp_dout(dpp, 0) << "D4NFilterObject::" << __func__ << "(): Failed to fetching attrs from backend store with ret: " << ret << dendl; return ret; } this->load_obj_state(dpp, y); - this->obj = *target_obj; + this->obj = next->get_obj(); if (!this->obj.key.instance.empty()) { this->set_instance(this->obj.key.instance); } @@ -2221,8 +2217,7 @@ int D4NFilterObject::D4NFilterReadOp::get_attr(const DoutPrefixProvider* dpp, co { rgw::sal::Attrs& attrs = source->get_attrs(); if (attrs.empty()) { - rgw_obj obj = source->get_obj(); - auto ret = source->get_obj_attrs(y, dpp, &obj); + auto ret = source->get_obj_attrs(y, dpp); if (ret < 0) { ldpp_dout(dpp, 0) << "D4NFilterObject::" << __func__ << "(): Error: failed to fetch attrs, ret=" << ret << dendl; return ret; diff --git a/src/rgw/driver/d4n/rgw_sal_d4n.h b/src/rgw/driver/d4n/rgw_sal_d4n.h index f844dc73d5fe..551ac1ca94ec 100644 --- a/src/rgw/driver/d4n/rgw_sal_d4n.h +++ b/src/rgw/driver/d4n/rgw_sal_d4n.h @@ -251,8 +251,7 @@ class D4NFilterObject : public FilterObject { bool follow_olh = true) override; virtual int set_obj_attrs(const DoutPrefixProvider* dpp, Attrs* setattrs, Attrs* delattrs, optional_yield y, uint32_t flags) override; - virtual int get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp, - rgw_obj* target_obj = NULL) override; + virtual int get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp) override; virtual int modify_obj_attrs(const char* attr_name, bufferlist& attr_val, optional_yield y, const DoutPrefixProvider* dpp, uint32_t flags = rgw::sal::FLAG_LOG_OP) override; diff --git a/src/rgw/driver/posix/rgw_sal_posix.cc b/src/rgw/driver/posix/rgw_sal_posix.cc index df625c8c0784..f7ca8fd06cba 100644 --- a/src/rgw/driver/posix/rgw_sal_posix.cc +++ b/src/rgw/driver/posix/rgw_sal_posix.cc @@ -2951,8 +2951,7 @@ int POSIXObject::set_obj_attrs(const DoutPrefixProvider* dpp, Attrs* setattrs, return 0; } -int POSIXObject::get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp, - rgw_obj* target_obj) +int POSIXObject::get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp) { //int fd; diff --git a/src/rgw/driver/posix/rgw_sal_posix.h b/src/rgw/driver/posix/rgw_sal_posix.h index 3af36d02a4c1..70a0106ea8d2 100644 --- a/src/rgw/driver/posix/rgw_sal_posix.h +++ b/src/rgw/driver/posix/rgw_sal_posix.h @@ -669,8 +669,7 @@ public: virtual int load_obj_state(const DoutPrefixProvider* dpp, optional_yield y, bool follow_olh = true) override; virtual int set_obj_attrs(const DoutPrefixProvider* dpp, Attrs* setattrs, Attrs* delattrs, optional_yield y, uint32_t flags) override; - virtual int get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp, - rgw_obj* target_obj = NULL) override; + virtual int get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp) override; virtual int modify_obj_attrs(const char* attr_name, bufferlist& attr_val, optional_yield y, const DoutPrefixProvider* dpp, uint32_t flags = rgw::sal::FLAG_LOG_OP) override; diff --git a/src/rgw/driver/rados/rgw_sal_rados.cc b/src/rgw/driver/rados/rgw_sal_rados.cc index 9a719bcbd946..8618868b3b13 100644 --- a/src/rgw/driver/rados/rgw_sal_rados.cc +++ b/src/rgw/driver/rados/rgw_sal_rados.cc @@ -2727,19 +2727,22 @@ int RadosObject::set_obj_attrs(const DoutPrefixProvider* dpp, Attrs* setattrs, A y, log_op, mtime); } -int RadosObject::get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp, rgw_obj* target_obj) +int RadosObject::get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp) { RGWRados::Object op_target(store->getRados(), bucket->get_info(), *rados_ctx, get_obj()); RGWRados::Object::Read read_op(&op_target); - return read_attrs(dpp, read_op, y, target_obj); + return read_attrs(dpp, read_op, y); } int RadosObject::modify_obj_attrs(const char* attr_name, bufferlist& attr_val, optional_yield y, const DoutPrefixProvider* dpp, uint32_t flags) { rgw_obj target = get_obj(); rgw_obj save = get_obj(); - int r = get_obj_attrs(y, dpp, &target); + RGWRados::Object op_target(store->getRados(), bucket->get_info(), *rados_ctx, get_obj()); + RGWRados::Object::Read read_op(&op_target); + + int r = read_attrs(dpp, read_op, y, &target); if (r < 0) { return r; } diff --git a/src/rgw/driver/rados/rgw_sal_rados.h b/src/rgw/driver/rados/rgw_sal_rados.h index 9e99d69b3b75..4bcbe73bfe0b 100644 --- a/src/rgw/driver/rados/rgw_sal_rados.h +++ b/src/rgw/driver/rados/rgw_sal_rados.h @@ -611,7 +611,7 @@ class RadosObject : public StoreObject { optional_yield y) override; virtual int set_obj_attrs(const DoutPrefixProvider* dpp, Attrs* setattrs, Attrs* delattrs, optional_yield y, uint32_t flags) override; - virtual int get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp, rgw_obj* target_obj = NULL) override; + virtual int get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp) override; virtual int modify_obj_attrs(const char* attr_name, bufferlist& attr_val, optional_yield y, const DoutPrefixProvider* dpp, uint32_t flags = rgw::sal::FLAG_LOG_OP) override; virtual int delete_obj_attrs(const DoutPrefixProvider* dpp, const char* attr_name, optional_yield y) override; diff --git a/src/rgw/rgw_sal.h b/src/rgw/rgw_sal.h index 44d61f1e2801..cb4f56314803 100644 --- a/src/rgw/rgw_sal.h +++ b/src/rgw/rgw_sal.h @@ -1231,7 +1231,7 @@ class Object { * deleted. @note the attribute APIs may be revisited in the future. */ virtual int set_obj_attrs(const DoutPrefixProvider* dpp, Attrs* setattrs, Attrs* delattrs, optional_yield y, uint32_t flags) = 0; /** Get attributes for this object */ - virtual int get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp, rgw_obj* target_obj = NULL) = 0; + virtual int get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp) = 0; /** Modify attributes for this object. */ virtual int modify_obj_attrs(const char* attr_name, bufferlist& attr_val, optional_yield y, const DoutPrefixProvider* dpp, uint32_t flags = rgw::sal::FLAG_LOG_OP) = 0; diff --git a/src/rgw/rgw_sal_dbstore.cc b/src/rgw/rgw_sal_dbstore.cc index d5e8b73d4dc5..29bafc75a6ce 100644 --- a/src/rgw/rgw_sal_dbstore.cc +++ b/src/rgw/rgw_sal_dbstore.cc @@ -544,18 +544,21 @@ namespace rgw::sal { return op_target.set_attrs(dpp, setattrs ? *setattrs : empty, delattrs); } - int DBObject::get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp, rgw_obj* target_obj) + int DBObject::get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp) { DB::Object op_target(store->getDB(), get_bucket()->get_info(), get_obj()); DB::Object::Read read_op(&op_target); - return read_attrs(dpp, read_op, y, target_obj); + return read_attrs(dpp, read_op, y, nullptr); } int DBObject::modify_obj_attrs(const char* attr_name, bufferlist& attr_val, optional_yield y, const DoutPrefixProvider* dpp, uint32_t flags) { rgw_obj target = get_obj(); - int r = get_obj_attrs(y, dpp, &target); + DB::Object op_target(store->getDB(), get_bucket()->get_info(), get_obj()); + DB::Object::Read read_op(&op_target); + + int r = read_attrs(dpp, read_op, y, &target); if (r < 0) { return r; } diff --git a/src/rgw/rgw_sal_dbstore.h b/src/rgw/rgw_sal_dbstore.h index 0173baf297cc..a25d4c478502 100644 --- a/src/rgw/rgw_sal_dbstore.h +++ b/src/rgw/rgw_sal_dbstore.h @@ -569,7 +569,7 @@ protected: bool is_sync_completed(const DoutPrefixProvider* dpp, optional_yield y, const ceph::real_time& obj_mtime) override; virtual int load_obj_state(const DoutPrefixProvider* dpp, optional_yield y, bool follow_olh = true) override; - virtual int get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp, rgw_obj* target_obj = NULL) override; + virtual int get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp) override; virtual int modify_obj_attrs(const char* attr_name, bufferlist& attr_val, optional_yield y, const DoutPrefixProvider* dpp, uint32_t flags = rgw::sal::FLAG_LOG_OP) override; virtual int delete_obj_attrs(const DoutPrefixProvider* dpp, const char* attr_name, optional_yield y) override; diff --git a/src/rgw/rgw_sal_filter.cc b/src/rgw/rgw_sal_filter.cc index 43fdb006c967..a54b7080ae21 100644 --- a/src/rgw/rgw_sal_filter.cc +++ b/src/rgw/rgw_sal_filter.cc @@ -1073,10 +1073,9 @@ int FilterObject::set_obj_attrs(const DoutPrefixProvider* dpp, Attrs* setattrs, return next->set_obj_attrs(dpp, setattrs, delattrs, y, flags); } -int FilterObject::get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp, - rgw_obj* target_obj) +int FilterObject::get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp) { - return next->get_obj_attrs(y, dpp, target_obj); + return next->get_obj_attrs(y, dpp); } int FilterObject::modify_obj_attrs(const char* attr_name, bufferlist& attr_val, diff --git a/src/rgw/rgw_sal_filter.h b/src/rgw/rgw_sal_filter.h index 9c37fd32fe0f..b370aaea4b4d 100644 --- a/src/rgw/rgw_sal_filter.h +++ b/src/rgw/rgw_sal_filter.h @@ -805,8 +805,7 @@ public: bool follow_olh = true) override; virtual int set_obj_attrs(const DoutPrefixProvider* dpp, Attrs* setattrs, Attrs* delattrs, optional_yield y, uint32_t flags) override; - virtual int get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp, - rgw_obj* target_obj = NULL) override; + virtual int get_obj_attrs(optional_yield y, const DoutPrefixProvider* dpp) override; virtual int modify_obj_attrs(const char* attr_name, bufferlist& attr_val, optional_yield y, const DoutPrefixProvider* dpp, uint32_t flags = rgw::sal::FLAG_LOG_OP) override; diff --git a/src/test/rgw/test_d4n_filter.cc b/src/test/rgw/test_d4n_filter.cc index 84de5141ccae..a707cf86c987 100755 --- a/src/test/rgw/test_d4n_filter.cc +++ b/src/test/rgw/test_d4n_filter.cc @@ -1685,8 +1685,7 @@ TEST_F(D4NFilterFixture, CopyNoneObjectWrite) EXPECT_EQ(testData, "test data"); // Ensure attr is not modified - rgw_obj copyObj = destObj->get_obj(); - ASSERT_EQ(destObj->get_obj_attrs(optional_yield{yield}, env->dpp, ©Obj), 0); + ASSERT_EQ(destObj->get_obj_attrs(optional_yield{yield}, env->dpp), 0); rgw::sal::Attrs copyAttrs = destObj->get_attrs(); buffer::list val = copyAttrs["user.rgw.test_attr"]; EXPECT_EQ(val.to_str(), "test_value"); @@ -1806,8 +1805,7 @@ TEST_F(D4NFilterFixture, CopyMergeObjectWrite) EXPECT_EQ(testData, "test data"); // Ensure attr is merged - rgw_obj copyObj = destObj->get_obj(); - ASSERT_EQ(destObj->get_obj_attrs(optional_yield{yield}, env->dpp, ©Obj), 0); + ASSERT_EQ(destObj->get_obj_attrs(optional_yield{yield}, env->dpp), 0); rgw::sal::Attrs copyAttrs = destObj->get_attrs(); buffer::list val = copyAttrs["user.rgw.test_attr"]; EXPECT_EQ(val.to_str(), "copy_value"); @@ -1927,8 +1925,7 @@ TEST_F(D4NFilterFixture, CopyReplaceObjectWrite) EXPECT_EQ(testData, "test data"); // Ensure attr is replaced - rgw_obj copyObj = destObj->get_obj(); - ASSERT_EQ(destObj->get_obj_attrs(optional_yield{yield}, env->dpp, ©Obj), 0); + ASSERT_EQ(destObj->get_obj_attrs(optional_yield{yield}, env->dpp), 0); rgw::sal::Attrs copyAttrs = destObj->get_attrs(); buffer::list val = copyAttrs["user.rgw.test_attr"]; EXPECT_EQ(val.to_str(), "copy_value"); @@ -2431,8 +2428,7 @@ TEST_F(D4NFilterFixture, CopyNoneVersionedObjectWrite) EXPECT_EQ(fs::exists(CACHE_DIR + "/" + TEST_BUCKET + testName + "/" + destNameEnabled + "/" + oid, err), true); // Ensure attr is not modified - rgw_obj copyObj = destObjEnabled->get_obj(); - ASSERT_EQ(destObjEnabled->get_obj_attrs(optional_yield{yield}, env->dpp, ©Obj), 0); + ASSERT_EQ(destObjEnabled->get_obj_attrs(optional_yield{yield}, env->dpp), 0); rgw::sal::Attrs copyAttrs = destObjEnabled->get_attrs(); buffer::list val = copyAttrs["user.rgw.test_attr"]; EXPECT_EQ(val.to_str(), "test_value"); @@ -2526,8 +2522,7 @@ TEST_F(D4NFilterFixture, CopyNoneVersionedObjectWrite) EXPECT_EQ(fs::exists(CACHE_DIR + "/" + TEST_BUCKET + testName + "/" + destNameSuspended + "/" + oid, err), true); // Ensure attr is not modified - rgw_obj copyObj = destObjSuspended->get_obj(); - ASSERT_EQ(destObjSuspended->get_obj_attrs(optional_yield{yield}, env->dpp, ©Obj), 0); + ASSERT_EQ(destObjSuspended->get_obj_attrs(optional_yield{yield}, env->dpp), 0); rgw::sal::Attrs copyAttrs = destObjSuspended->get_attrs(); buffer::list val = copyAttrs["user.rgw.test_attr"]; EXPECT_EQ(val.to_str(), "test_value"); @@ -2651,8 +2646,7 @@ TEST_F(D4NFilterFixture, CopyMergeVersionedObjectWrite) EXPECT_EQ(fs::exists(CACHE_DIR + "/" + TEST_BUCKET + testName + "/" + destNameEnabled + "/" + oid, err), true); // Ensure attr is merged - rgw_obj copyObj = destObjEnabled->get_obj(); - ASSERT_EQ(destObjEnabled->get_obj_attrs(optional_yield{yield}, env->dpp, ©Obj), 0); + ASSERT_EQ(destObjEnabled->get_obj_attrs(optional_yield{yield}, env->dpp), 0); rgw::sal::Attrs copyAttrs = destObjEnabled->get_attrs(); buffer::list val = copyAttrs["user.rgw.test_attr"]; EXPECT_EQ(val.to_str(), "copy_value"); @@ -2746,8 +2740,7 @@ TEST_F(D4NFilterFixture, CopyMergeVersionedObjectWrite) EXPECT_EQ(fs::exists(CACHE_DIR + "/" + TEST_BUCKET + testName + "/" + destNameSuspended + "/" + oid, err), true); // Ensure attr is merged - rgw_obj copyObj = destObjSuspended->get_obj(); - ASSERT_EQ(destObjSuspended->get_obj_attrs(optional_yield{yield}, env->dpp, ©Obj), 0); + ASSERT_EQ(destObjSuspended->get_obj_attrs(optional_yield{yield}, env->dpp), 0); rgw::sal::Attrs copyAttrs = destObjSuspended->get_attrs(); buffer::list val = copyAttrs["user.rgw.test_attr"]; EXPECT_EQ(val.to_str(), "copy_value"); @@ -2871,8 +2864,7 @@ TEST_F(D4NFilterFixture, CopyReplaceVersionedObjectWrite) EXPECT_EQ(fs::exists(CACHE_DIR + "/" + TEST_BUCKET + testName + "/" + destNameEnabled + "/" + oid, err), true); // Ensure attr is replaced - rgw_obj copyObj = destObjEnabled->get_obj(); - ASSERT_EQ(destObjEnabled->get_obj_attrs(optional_yield{yield}, env->dpp, ©Obj), 0); + ASSERT_EQ(destObjEnabled->get_obj_attrs(optional_yield{yield}, env->dpp), 0); rgw::sal::Attrs copyAttrs = destObjEnabled->get_attrs(); buffer::list val = copyAttrs["user.rgw.test_attr"]; EXPECT_EQ(val.to_str(), "copy_value"); @@ -2966,8 +2958,7 @@ TEST_F(D4NFilterFixture, CopyReplaceVersionedObjectWrite) EXPECT_EQ(fs::exists(CACHE_DIR + "/" + TEST_BUCKET + testName + "/" + destNameSuspended + "/" + oid, err), true); // Ensure attr is replaced - rgw_obj copyObj = destObjSuspended->get_obj(); - ASSERT_EQ(destObjSuspended->get_obj_attrs(optional_yield{yield}, env->dpp, ©Obj), 0); + ASSERT_EQ(destObjSuspended->get_obj_attrs(optional_yield{yield}, env->dpp), 0); rgw::sal::Attrs copyAttrs = destObjSuspended->get_attrs(); buffer::list val = copyAttrs["user.rgw.test_attr"]; EXPECT_EQ(val.to_str(), "copy_value");