From 0b02adfa78457ce6b74f7b9ffb1ab102499d6ab1 Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Mon, 18 Sep 2023 11:15:02 -0400 Subject: [PATCH] rgw/sal: get_placement_target_names() returns void the function returned an integer error code, but two callers were incorrectly testing the return value as a boolean the function just returns placement ids that are in-memory, so none of the drivers have a failure case; change the return value to void Fixes: https://tracker.ceph.com/issues/62771 Signed-off-by: Casey Bodley (cherry picked from commit 3ad17ed3b222701bb6f4e3150989584645789d7c) --- src/rgw/driver/rados/rgw_sal_rados.cc | 4 +--- src/rgw/driver/rados/rgw_sal_rados.h | 2 +- src/rgw/rgw_op.cc | 7 +++---- src/rgw/rgw_rest_swift.cc | 15 +++++++-------- src/rgw/rgw_sal.h | 2 +- src/rgw/rgw_sal_daos.cc | 4 +--- src/rgw/rgw_sal_daos.h | 2 +- src/rgw/rgw_sal_dbstore.cc | 4 +--- src/rgw/rgw_sal_dbstore.h | 2 +- src/rgw/rgw_sal_filter.h | 4 ++-- src/rgw/rgw_sal_motr.cc | 4 +--- src/rgw/rgw_sal_motr.h | 2 +- 12 files changed, 21 insertions(+), 31 deletions(-) diff --git a/src/rgw/driver/rados/rgw_sal_rados.cc b/src/rgw/driver/rados/rgw_sal_rados.cc index 608367b8a85dd..9acdb79d38753 100644 --- a/src/rgw/driver/rados/rgw_sal_rados.cc +++ b/src/rgw/driver/rados/rgw_sal_rados.cc @@ -3191,13 +3191,11 @@ bool RadosZoneGroup::placement_target_exists(std::string& target) const return !!group.placement_targets.count(target); } -int RadosZoneGroup::get_placement_target_names(std::set& names) const +void RadosZoneGroup::get_placement_target_names(std::set& names) const { for (const auto& target : group.placement_targets) { names.emplace(target.second.name); } - - return 0; } int RadosZoneGroup::get_placement_tier(const rgw_placement_rule& rule, diff --git a/src/rgw/driver/rados/rgw_sal_rados.h b/src/rgw/driver/rados/rgw_sal_rados.h index d2ba6300e8141..4d2dc97091ef4 100644 --- a/src/rgw/driver/rados/rgw_sal_rados.h +++ b/src/rgw/driver/rados/rgw_sal_rados.h @@ -70,7 +70,7 @@ public: return group.is_master_zonegroup(); }; virtual const std::string& get_api_name() const override { return group.api_name; }; - virtual int get_placement_target_names(std::set& names) const override; + virtual void get_placement_target_names(std::set& names) const override; virtual const std::string& get_default_placement_name() const override { return group.default_placement.name; }; virtual int get_hostnames(std::list& names) const override { diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc index df09223ca909b..6a1a39dadc5c6 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -2529,10 +2529,9 @@ void RGWListBuckets::execute(optional_yield y) * isn't actually used in a given account. In such situation its usage * stats would be simply full of zeros. */ std::set targets; - if (driver->get_zone()->get_zonegroup().get_placement_target_names(targets)) { - for (const auto& policy : targets) { - policies_stats.emplace(policy, decltype(policies_stats)::mapped_type()); - } + driver->get_zone()->get_zonegroup().get_placement_target_names(targets); + for (const auto& policy : targets) { + policies_stats.emplace(policy, decltype(policies_stats)::mapped_type()); } std::map>& m = buckets.get_buckets(); diff --git a/src/rgw/rgw_rest_swift.cc b/src/rgw/rgw_rest_swift.cc index a9565817005dd..741b9c8d2082e 100644 --- a/src/rgw/rgw_rest_swift.cc +++ b/src/rgw/rgw_rest_swift.cc @@ -1897,14 +1897,13 @@ void RGWInfo_ObjStore_SWIFT::list_swift_data(Formatter& formatter, const rgw::sal::ZoneGroup& zonegroup = driver->get_zone()->get_zonegroup(); std::set targets; - if (zonegroup.get_placement_target_names(targets)) { - for (const auto& placement_targets : targets) { - formatter.open_object_section("policy"); - if (placement_targets.compare(zonegroup.get_default_placement_name()) == 0) - formatter.dump_bool("default", true); - formatter.dump_string("name", placement_targets.c_str()); - formatter.close_section(); - } + zonegroup.get_placement_target_names(targets); + for (const auto& placement_targets : targets) { + formatter.open_object_section("policy"); + if (placement_targets.compare(zonegroup.get_default_placement_name()) == 0) + formatter.dump_bool("default", true); + formatter.dump_string("name", placement_targets.c_str()); + formatter.close_section(); } formatter.close_section(); diff --git a/src/rgw/rgw_sal.h b/src/rgw/rgw_sal.h index 9b379572d8c2b..944737dee7fb1 100644 --- a/src/rgw/rgw_sal.h +++ b/src/rgw/rgw_sal.h @@ -1483,7 +1483,7 @@ public: /** Get the API name of this zonegroup */ virtual const std::string& get_api_name() const = 0; /** Get the list of placement target names for this zone */ - virtual int get_placement_target_names(std::set& names) const = 0; + virtual void get_placement_target_names(std::set& names) const = 0; /** Get the name of the default placement target for this zone */ virtual const std::string& get_default_placement_name() const = 0; /** Get the list of hostnames from this zone */ diff --git a/src/rgw/rgw_sal_daos.cc b/src/rgw/rgw_sal_daos.cc index 88b1173e5605d..4b0234b1f30fc 100644 --- a/src/rgw/rgw_sal_daos.cc +++ b/src/rgw/rgw_sal_daos.cc @@ -826,13 +826,11 @@ bool DaosZoneGroup::placement_target_exists(std::string& target) const { return !!group.placement_targets.count(target); } -int DaosZoneGroup::get_placement_target_names( +void DaosZoneGroup::get_placement_target_names( std::set& names) const { for (const auto& target : group.placement_targets) { names.emplace(target.second.name); } - - return 0; } int DaosZoneGroup::get_placement_tier(const rgw_placement_rule& rule, diff --git a/src/rgw/rgw_sal_daos.h b/src/rgw/rgw_sal_daos.h index b381156f99318..64bf49c7c2528 100644 --- a/src/rgw/rgw_sal_daos.h +++ b/src/rgw/rgw_sal_daos.h @@ -414,7 +414,7 @@ class DaosZoneGroup : public StoreZoneGroup { virtual const std::string& get_api_name() const override { return group.api_name; }; - virtual int get_placement_target_names( + virtual void get_placement_target_names( std::set& names) const override; virtual const std::string& get_default_placement_name() const override { return group.default_placement.name; diff --git a/src/rgw/rgw_sal_dbstore.cc b/src/rgw/rgw_sal_dbstore.cc index d9f44dbaf711c..5100dc41efe5c 100644 --- a/src/rgw/rgw_sal_dbstore.cc +++ b/src/rgw/rgw_sal_dbstore.cc @@ -531,12 +531,10 @@ namespace rgw::sal { return !!group->placement_targets.count(target); } - int DBZoneGroup::get_placement_target_names(std::set& names) const { + void DBZoneGroup::get_placement_target_names(std::set& names) const { for (const auto& target : group->placement_targets) { names.emplace(target.second.name); } - - return 0; } ZoneGroup& DBZone::get_zonegroup() diff --git a/src/rgw/rgw_sal_dbstore.h b/src/rgw/rgw_sal_dbstore.h index 7fefd20afbdfa..3acdb4ba3add1 100644 --- a/src/rgw/rgw_sal_dbstore.h +++ b/src/rgw/rgw_sal_dbstore.h @@ -271,7 +271,7 @@ protected: return group->is_master_zonegroup(); }; virtual const std::string& get_api_name() const override { return group->api_name; }; - virtual int get_placement_target_names(std::set& names) const override; + virtual void get_placement_target_names(std::set& names) const override; virtual const std::string& get_default_placement_name() const override { return group->default_placement.name; }; virtual int get_hostnames(std::list& names) const override { diff --git a/src/rgw/rgw_sal_filter.h b/src/rgw/rgw_sal_filter.h index a01c1e56c6ed4..951a1de5fc40b 100644 --- a/src/rgw/rgw_sal_filter.h +++ b/src/rgw/rgw_sal_filter.h @@ -68,8 +68,8 @@ public: { return next->is_master_zonegroup(); } virtual const std::string& get_api_name() const override { return next->get_api_name(); } - virtual int get_placement_target_names(std::set& names) const override - { return next->get_placement_target_names(names); } + virtual void get_placement_target_names(std::set& names) const override + { next->get_placement_target_names(names); } virtual const std::string& get_default_placement_name() const override { return next->get_default_placement_name(); } virtual int get_hostnames(std::list& names) const override diff --git a/src/rgw/rgw_sal_motr.cc b/src/rgw/rgw_sal_motr.cc index cc7869627a666..de18ba9447c94 100644 --- a/src/rgw/rgw_sal_motr.cc +++ b/src/rgw/rgw_sal_motr.cc @@ -1081,13 +1081,11 @@ bool MotrZoneGroup::placement_target_exists(std::string& target) const return !!group.placement_targets.count(target); } -int MotrZoneGroup::get_placement_target_names(std::set& names) const +void MotrZoneGroup::get_placement_target_names(std::set& names) const { for (const auto& target : group.placement_targets) { names.emplace(target.second.name); } - - return 0; } int MotrZoneGroup::get_placement_tier(const rgw_placement_rule& rule, diff --git a/src/rgw/rgw_sal_motr.h b/src/rgw/rgw_sal_motr.h index f2dfda2c510cb..b7230f7e1fd3e 100644 --- a/src/rgw/rgw_sal_motr.h +++ b/src/rgw/rgw_sal_motr.h @@ -445,7 +445,7 @@ public: return group.is_master_zonegroup(); }; virtual const std::string& get_api_name() const override { return group.api_name; }; - virtual int get_placement_target_names(std::set& names) const override; + virtual void get_placement_target_names(std::set& names) const override; virtual const std::string& get_default_placement_name() const override { return group.default_placement.name; }; virtual int get_hostnames(std::list& names) const override { -- 2.39.5