From 65864c2006702aecc71b70f2214af86209207e01 Mon Sep 17 00:00:00 2001 From: Daniel Gryniewicz Date: Fri, 20 Aug 2021 12:05:00 -0400 Subject: [PATCH] RGW Zipper - don't reference instance in bucket APIs Signed-off-by: Daniel Gryniewicz --- src/rgw/rgw_admin.cc | 4 ++-- src/rgw/rgw_bucket.cc | 6 ++--- src/rgw/rgw_lc.cc | 4 ++-- src/rgw/rgw_op.cc | 46 +++++++++++++++++++------------------- src/rgw/rgw_sal.h | 11 +++++++-- src/rgw/rgw_sal_dbstore.cc | 8 ++++--- src/rgw/rgw_sal_dbstore.h | 4 ++-- src/rgw/rgw_sal_rados.cc | 9 ++++---- src/rgw/rgw_sal_rados.h | 4 ++-- 9 files changed, 53 insertions(+), 43 deletions(-) diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc index 71ba340d12b5e..09d39433984ec 100644 --- a/src/rgw/rgw_admin.cc +++ b/src/rgw/rgw_admin.cc @@ -1313,7 +1313,7 @@ int set_bucket_quota(rgw::sal::Store* store, OPT opt_cmd, set_quota_info(bucket->get_info().quota, opt_cmd, max_size, max_objects, have_max_size, have_max_objects); - r = bucket->put_instance_info(dpp(), false, real_time()); + r = bucket->put_info(dpp(), false, real_time()); if (r < 0) { cerr << "ERROR: failed writing bucket instance info: " << cpp_strerror(-r) << std::endl; return -r; @@ -2918,7 +2918,7 @@ public: return 0; } - int ret = bucket->put_instance_info(dpp(), false, real_time()); + int ret = bucket->put_info(dpp(), false, real_time()); if (ret < 0) { cerr << "failed to store bucket info: " << cpp_strerror(-ret) << std::endl; return -ret; diff --git a/src/rgw/rgw_bucket.cc b/src/rgw/rgw_bucket.cc index bc34ec821aa70..edc7f8027311f 100644 --- a/src/rgw/rgw_bucket.cc +++ b/src/rgw/rgw_bucket.cc @@ -422,7 +422,7 @@ int RGWBucket::set_quota(RGWBucketAdminOpState& op_state, const DoutPrefixProvid bucket = op_state.get_bucket()->clone(); bucket->get_info().quota = op_state.quota; - int r = bucket->put_instance_info(dpp, false, real_time()); + int r = bucket->put_info(dpp, false, real_time()); if (r < 0) { set_err_msg(err_msg, "ERROR: failed writing bucket instance info: " + cpp_strerror(-r)); return r; @@ -663,7 +663,7 @@ int RGWBucket::sync(RGWBucketAdminOpState& op_state, const DoutPrefixProvider *d bucket->get_info().flags |= BUCKET_DATASYNC_DISABLED; } - int r = bucket->put_instance_info(dpp, false, real_time()); + int r = bucket->put_info(dpp, false, real_time()); if (r < 0) { set_err_msg(err_msg, "ERROR: failed writing bucket instance info:" + cpp_strerror(-r)); return r; @@ -911,7 +911,7 @@ int RGWBucketAdminOp::link(rgw::sal::Store* store, RGWBucketAdminOpState& op_sta exclusive = true; } - r = loc_bucket->put_instance_info(dpp, exclusive, ceph::real_time()); + r = loc_bucket->put_info(dpp, exclusive, ceph::real_time()); if (r < 0) { set_err_msg(err, "ERROR: failed writing bucket instance info: " + cpp_strerror(-r)); return r; diff --git a/src/rgw/rgw_lc.cc b/src/rgw/rgw_lc.cc index 3f186a5c970db..485848d22975a 100644 --- a/src/rgw/rgw_lc.cc +++ b/src/rgw/rgw_lc.cc @@ -2009,7 +2009,7 @@ int RGWLC::set_bucket_config(rgw::sal::Bucket* bucket, attrs[RGW_ATTR_LC] = std::move(lc_bl); int ret = - bucket->set_instance_attrs(this, attrs, null_yield); + bucket->merge_and_store_attrs(this, attrs, null_yield); if (ret < 0) return ret; @@ -2030,7 +2030,7 @@ int RGWLC::remove_bucket_config(rgw::sal::Bucket* bucket, { rgw::sal::Attrs attrs = bucket_attrs; attrs.erase(RGW_ATTR_LC); - int ret = bucket->set_instance_attrs(this, attrs, null_yield); + int ret = bucket->merge_and_store_attrs(this, attrs, null_yield); rgw_bucket& b = bucket->get_key(); diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc index 6c9d19e43ec37..1a04e5c21eb0e 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -1064,7 +1064,7 @@ void RGWPutBucketTags::execute(optional_yield y) op_ret = retry_raced_bucket_write(this, s->bucket.get(), [this, y] { rgw::sal::Attrs attrs = s->bucket->get_attrs(); attrs[RGW_ATTR_TAGS] = tags_bl; - return s->bucket->set_instance_attrs(this, attrs, y); + return s->bucket->merge_and_store_attrs(this, attrs, y); }); } @@ -1091,7 +1091,7 @@ void RGWDeleteBucketTags::execute(optional_yield y) op_ret = retry_raced_bucket_write(this, s->bucket.get(), [this, y] { rgw::sal::Attrs attrs = s->bucket->get_attrs(); attrs.erase(RGW_ATTR_TAGS); - op_ret = s->bucket->set_instance_attrs(this, attrs, y); + op_ret = s->bucket->merge_and_store_attrs(this, attrs, y); if (op_ret < 0) { ldpp_dout(this, 0) << "RGWDeleteBucketTags() failed to remove RGW_ATTR_TAGS on bucket=" << s->bucket->get_name() @@ -1145,7 +1145,7 @@ void RGWPutBucketReplication::execute(optional_yield y) { s->bucket->get_info().set_sync_policy(std::move(sync_policy)); - int ret = s->bucket->put_instance_info(this, false, real_time()); + int ret = s->bucket->put_info(this, false, real_time()); if (ret < 0) { ldpp_dout(this, 0) << "ERROR: put_bucket_instance_info (bucket=" << s->bucket << ") returned ret=" << ret << dendl; return ret; @@ -1185,7 +1185,7 @@ void RGWDeleteBucketReplication::execute(optional_yield y) s->bucket->get_info().set_sync_policy(std::move(sync_policy)); - int ret = s->bucket->put_instance_info(this, false, real_time()); + int ret = s->bucket->put_info(this, false, real_time()); if (ret < 0) { ldpp_dout(this, 0) << "ERROR: put_bucket_instance_info (bucket=" << s->bucket << ") returned ret=" << ret << dendl; return ret; @@ -2535,7 +2535,7 @@ void RGWSetBucketVersioning::execute(optional_yield y) return op_ret; } s->bucket->set_attrs(rgw::sal::Attrs(s->bucket_attrs)); - return s->bucket->put_instance_info(this, false, real_time()); + return s->bucket->put_info(this, false, real_time()); }); if (!modified) { @@ -2597,7 +2597,7 @@ void RGWSetBucketWebsite::execute(optional_yield y) op_ret = retry_raced_bucket_write(this, s->bucket.get(), [this] { s->bucket->get_info().has_website = true; s->bucket->get_info().website_conf = website_conf; - op_ret = s->bucket->put_instance_info(this, false, real_time()); + op_ret = s->bucket->put_info(this, false, real_time()); return op_ret; }); @@ -2636,7 +2636,7 @@ void RGWDeleteBucketWebsite::execute(optional_yield y) op_ret = retry_raced_bucket_write(this, s->bucket.get(), [this] { s->bucket->get_info().has_website = false; s->bucket->get_info().website_conf = RGWBucketWebsiteConf(); - op_ret = s->bucket->put_instance_info(this, false, real_time()); + op_ret = s->bucket->put_info(this, false, real_time()); return op_ret; }); if (op_ret < 0) { @@ -3163,7 +3163,7 @@ void RGWCreateBucket::execute(optional_yield y) s->bucket->get_info().has_website = !s->bucket->get_info().website_conf.is_empty(); /* This will also set the quota on the bucket. */ - op_ret = s->bucket->set_instance_attrs(this, attrs, y); + op_ret = s->bucket->merge_and_store_attrs(this, attrs, y); } while (op_ret == -ECANCELED && tries++ < 20); /* Restore the proper return code. */ @@ -4493,7 +4493,7 @@ void RGWPutMetadataBucket::execute(optional_yield y) /* Setting attributes also stores the provided bucket info. Due * to this fact, the new quota settings can be serialized with * the same call. */ - op_ret = s->bucket->set_instance_attrs(this, attrs, s->yield); + op_ret = s->bucket->merge_and_store_attrs(this, attrs, s->yield); return op_ret; }); } @@ -5520,7 +5520,7 @@ void RGWPutACLs::execute(optional_yield y) } else { map attrs = s->bucket_attrs; attrs[RGW_ATTR_ACL] = bl; - op_ret = s->bucket->set_instance_attrs(this, attrs, y); + op_ret = s->bucket->merge_and_store_attrs(this, attrs, y); } if (op_ret == -ECANCELED) { op_ret = 0; /* lost a race, but it's ok because acls are immutable */ @@ -5675,7 +5675,7 @@ void RGWPutCORS::execute(optional_yield y) op_ret = retry_raced_bucket_write(this, s->bucket.get(), [this] { rgw::sal::Attrs attrs(s->bucket_attrs); attrs[RGW_ATTR_CORS] = cors_bl; - return s->bucket->set_instance_attrs(this, attrs, s->yield); + return s->bucket->merge_and_store_attrs(this, attrs, s->yield); }); } @@ -5707,7 +5707,7 @@ void RGWDeleteCORS::execute(optional_yield y) rgw::sal::Attrs attrs(s->bucket_attrs); attrs.erase(RGW_ATTR_CORS); - op_ret = s->bucket->set_instance_attrs(this, attrs, s->yield); + op_ret = s->bucket->merge_and_store_attrs(this, attrs, s->yield); if (op_ret < 0) { ldpp_dout(this, 0) << "RGWLC::RGWDeleteCORS() failed to set attrs on bucket=" << s->bucket->get_name() << " returned err=" << op_ret << dendl; @@ -5810,7 +5810,7 @@ void RGWSetRequestPayment::execute(optional_yield y) return; s->bucket->get_info().requester_pays = requester_pays; - op_ret = s->bucket->put_instance_info(this, false, real_time()); + op_ret = s->bucket->put_info(this, false, real_time()); if (op_ret < 0) { ldpp_dout(this, 0) << "NOTICE: put_bucket_info on bucket=" << s->bucket->get_name() << " returned err=" << op_ret << dendl; @@ -7465,7 +7465,7 @@ void RGWSetAttrs::execute(optional_yield y) rgw::sal::Attrs a(attrs); op_ret = s->object->set_obj_attrs(this, s->obj_ctx, &a, nullptr, y); } else { - op_ret = s->bucket->set_instance_attrs(this, attrs, y); + op_ret = s->bucket->merge_and_store_attrs(this, attrs, y); } } /* RGWSetAttrs::execute() */ @@ -7506,7 +7506,7 @@ void RGWConfigBucketMetaSearch::execute(optional_yield y) s->bucket->get_info().mdsearch_config = mdsearch_config; - op_ret = s->bucket->put_instance_info(this, false, real_time()); + op_ret = s->bucket->put_info(this, false, real_time()); if (op_ret < 0) { ldpp_dout(this, 0) << "NOTICE: put_bucket_info on bucket=" << s->bucket->get_name() << " returned err=" << op_ret << dendl; @@ -7547,7 +7547,7 @@ void RGWDelBucketMetaSearch::execute(optional_yield y) { s->bucket->get_info().mdsearch_config.clear(); - op_ret = s->bucket->put_instance_info(this, false, real_time()); + op_ret = s->bucket->put_info(this, false, real_time()); if (op_ret < 0) { ldpp_dout(this, 0) << "NOTICE: put_bucket_info on bucket=" << s->bucket->get_name() << " returned err=" << op_ret << dendl; @@ -7687,7 +7687,7 @@ void RGWPutBucketPolicy::execute(optional_yield y) op_ret = retry_raced_bucket_write(this, s->bucket.get(), [&p, this, &attrs] { attrs[RGW_ATTR_IAM_POLICY].clear(); attrs[RGW_ATTR_IAM_POLICY].append(p.text); - op_ret = s->bucket->set_instance_attrs(this, attrs, s->yield); + op_ret = s->bucket->merge_and_store_attrs(this, attrs, s->yield); return op_ret; }); } catch (rgw::IAM::PolicyParseException& e) { @@ -7768,7 +7768,7 @@ void RGWDeleteBucketPolicy::execute(optional_yield y) op_ret = retry_raced_bucket_write(this, s->bucket.get(), [this] { rgw::sal::Attrs attrs(s->bucket_attrs); attrs.erase(RGW_ATTR_IAM_POLICY); - op_ret = s->bucket->set_instance_attrs(this, attrs, s->yield); + op_ret = s->bucket->merge_and_store_attrs(this, attrs, s->yield); return op_ret; }); } @@ -7829,7 +7829,7 @@ void RGWPutBucketObjectLock::execute(optional_yield y) op_ret = retry_raced_bucket_write(this, s->bucket.get(), [this] { s->bucket->get_info().obj_lock = obj_lock; - op_ret = s->bucket->put_instance_info(this, false, real_time()); + op_ret = s->bucket->put_info(this, false, real_time()); return op_ret; }); return; @@ -8166,7 +8166,7 @@ void RGWPutBucketPublicAccessBlock::execute(optional_yield y) op_ret = retry_raced_bucket_write(this, s->bucket.get(), [this, &bl] { rgw::sal::Attrs attrs(s->bucket_attrs); attrs[RGW_ATTR_PUBLIC_ACCESS] = bl; - return s->bucket->set_instance_attrs(this, attrs, s->yield); + return s->bucket->merge_and_store_attrs(this, attrs, s->yield); }); } @@ -8232,7 +8232,7 @@ void RGWDeleteBucketPublicAccessBlock::execute(optional_yield y) op_ret = retry_raced_bucket_write(this, s->bucket.get(), [this] { rgw::sal::Attrs attrs(s->bucket_attrs); attrs.erase(RGW_ATTR_PUBLIC_ACCESS); - op_ret = s->bucket->set_instance_attrs(this, attrs, s->yield); + op_ret = s->bucket->merge_and_store_attrs(this, attrs, s->yield); return op_ret; }); } @@ -8308,7 +8308,7 @@ void RGWPutBucketEncryption::execute(optional_yield y) rgw::sal::Attrs attrs = s->bucket->get_attrs(); attrs[RGW_ATTR_BUCKET_ENCRYPTION_POLICY] = conf_bl; attrs[RGW_ATTR_BUCKET_ENCRYPTION_KEY_ID] = key_id_bl; - return s->bucket->set_instance_attrs(this, attrs, y); + return s->bucket->merge_and_store_attrs(this, attrs, y); }); } @@ -8362,7 +8362,7 @@ void RGWDeleteBucketEncryption::execute(optional_yield y) rgw::sal::Attrs attrs = s->bucket->get_attrs(); attrs.erase(RGW_ATTR_BUCKET_ENCRYPTION_POLICY); attrs.erase(RGW_ATTR_BUCKET_ENCRYPTION_KEY_ID); - op_ret = s->bucket->set_instance_attrs(this, attrs, y); + op_ret = s->bucket->merge_and_store_attrs(this, attrs, y); return op_ret; }); } diff --git a/src/rgw/rgw_sal.h b/src/rgw/rgw_sal.h index fcd8de14eab70..92c2bddee6111 100644 --- a/src/rgw/rgw_sal.h +++ b/src/rgw/rgw_sal.h @@ -441,14 +441,21 @@ class Bucket { virtual int update_container_stats(const DoutPrefixProvider* dpp) = 0; virtual int check_bucket_shards(const DoutPrefixProvider* dpp) = 0; virtual int chown(const DoutPrefixProvider* dpp, User* new_user, User* old_user, optional_yield y, const std::string* marker = nullptr) = 0; - virtual int put_instance_info(const DoutPrefixProvider* dpp, bool exclusive, ceph::real_time mtime) = 0; + virtual int put_info(const DoutPrefixProvider* dpp, bool exclusive, ceph::real_time mtime) = 0; virtual int remove_metadata(const DoutPrefixProvider* dpp, RGWObjVersionTracker* objv, optional_yield y) = 0; virtual bool is_owner(User* user) = 0; virtual User* get_owner(void) { return owner; }; virtual ACLOwner get_acl_owner(void) { return ACLOwner(info.owner); }; virtual int check_empty(const DoutPrefixProvider* dpp, optional_yield y) = 0; virtual int check_quota(const DoutPrefixProvider *dpp, RGWQuotaInfo& user_quota, RGWQuotaInfo& bucket_quota, uint64_t obj_size, optional_yield y, bool check_size_only = false) = 0; - virtual int set_instance_attrs(const DoutPrefixProvider* dpp, Attrs& attrs, optional_yield y) = 0; + /** Set the attributes in attrs, leaving any other existing attrs set, and + * write them to the backing store; a merge operation */ + virtual int merge_and_store_attrs(const DoutPrefixProvider* dpp, Attrs& new_attrs, optional_yield y) { + for(auto& it : new_attrs) { + attrs[it.first] = it.second; + } + return 0; + } virtual int try_refresh_info(const DoutPrefixProvider* dpp, ceph::real_time* pmtime) = 0; virtual int read_usage(const DoutPrefixProvider *dpp, uint64_t start_epoch, uint64_t end_epoch, uint32_t max_entries, bool* is_truncated, RGWUsageIter& usage_iter, diff --git a/src/rgw/rgw_sal_dbstore.cc b/src/rgw/rgw_sal_dbstore.cc index 99dc875316e00..dcb968e6c34c4 100644 --- a/src/rgw/rgw_sal_dbstore.cc +++ b/src/rgw/rgw_sal_dbstore.cc @@ -206,7 +206,7 @@ namespace rgw::sal { return ret; } - int DBBucket::put_instance_info(const DoutPrefixProvider *dpp, bool exclusive, ceph::real_time _mtime) + int DBBucket::put_info(const DoutPrefixProvider *dpp, bool exclusive, ceph::real_time _mtime) { int ret; @@ -245,13 +245,15 @@ namespace rgw::sal { return 0; } - int DBBucket::set_instance_attrs(const DoutPrefixProvider *dpp, Attrs& attrs, optional_yield y) + int DBBucket::merge_and_store_attrs(const DoutPrefixProvider *dpp, Attrs& new_attrs, optional_yield y) { int ret = 0; + Bucket::merge_and_store_attrs(dpp, new_attrs, y); + /* XXX: handle has_instance_obj like in set_bucket_instance_attrs() */ - ret = store->getDB()->update_bucket(dpp, "attrs", info, false, nullptr, &attrs, nullptr, &get_info().objv_tracker); + ret = store->getDB()->update_bucket(dpp, "attrs", info, false, nullptr, &new_attrs, nullptr, &get_info().objv_tracker); return ret; } diff --git a/src/rgw/rgw_sal_dbstore.h b/src/rgw/rgw_sal_dbstore.h index 01eef17246d60..2b95af27216e4 100644 --- a/src/rgw/rgw_sal_dbstore.h +++ b/src/rgw/rgw_sal_dbstore.h @@ -140,12 +140,12 @@ namespace rgw { namespace sal { virtual int update_container_stats(const DoutPrefixProvider *dpp) override; virtual int check_bucket_shards(const DoutPrefixProvider *dpp) override; virtual int chown(const DoutPrefixProvider *dpp, User* new_user, User* old_user, optional_yield y, const std::string* marker = nullptr) override; - virtual int put_instance_info(const DoutPrefixProvider *dpp, bool exclusive, ceph::real_time mtime) override; + virtual int put_info(const DoutPrefixProvider *dpp, bool exclusive, ceph::real_time mtime) override; virtual int remove_metadata(const DoutPrefixProvider* dpp, RGWObjVersionTracker* objv, optional_yield y) override; virtual bool is_owner(User* user) override; virtual int check_empty(const DoutPrefixProvider *dpp, optional_yield y) override; virtual int check_quota(const DoutPrefixProvider *dpp, RGWQuotaInfo& user_quota, RGWQuotaInfo& bucket_quota, uint64_t obj_size, optional_yield y, bool check_size_only = false) override; - virtual int set_instance_attrs(const DoutPrefixProvider *dpp, Attrs& attrs, optional_yield y) override; + virtual int merge_and_store_attrs(const DoutPrefixProvider *dpp, Attrs& attrs, optional_yield y) override; virtual int try_refresh_info(const DoutPrefixProvider *dpp, ceph::real_time *pmtime) override; virtual int read_usage(const DoutPrefixProvider *dpp, uint64_t start_epoch, uint64_t end_epoch, uint32_t max_entries, bool *is_truncated, RGWUsageIter& usage_iter, diff --git a/src/rgw/rgw_sal_rados.cc b/src/rgw/rgw_sal_rados.cc index 74b9960667c17..11428f1363ce7 100644 --- a/src/rgw/rgw_sal_rados.cc +++ b/src/rgw/rgw_sal_rados.cc @@ -594,7 +594,7 @@ int RadosBucket::chown(const DoutPrefixProvider* dpp, User* new_user, User* old_ old_user->get_display_name(), *marker, y, dpp); } -int RadosBucket::put_instance_info(const DoutPrefixProvider* dpp, bool exclusive, ceph::real_time _mtime) +int RadosBucket::put_info(const DoutPrefixProvider* dpp, bool exclusive, ceph::real_time _mtime) { mtime = _mtime; return store->getRados()->put_bucket_instance_info(info, exclusive, mtime, &attrs, dpp); @@ -631,10 +631,11 @@ int RadosBucket::check_quota(const DoutPrefixProvider *dpp, RGWQuotaInfo& user_q user_quota, bucket_quota, obj_size, y, check_size_only); } -int RadosBucket::set_instance_attrs(const DoutPrefixProvider* dpp, Attrs& attrs, optional_yield y) +int RadosBucket::merge_and_store_attrs(const DoutPrefixProvider* dpp, Attrs& new_attrs, optional_yield y) { - return store->ctl()->bucket->set_bucket_instance_attrs(get_info(), - attrs, &get_info().objv_tracker, y, dpp); + Bucket::merge_and_store_attrs(dpp, new_attrs, y); + return store->ctl()->bucket->set_bucket_instance_attrs(get_info(), + new_attrs, &get_info().objv_tracker, y, dpp); } int RadosBucket::try_refresh_info(const DoutPrefixProvider* dpp, ceph::real_time* pmtime) diff --git a/src/rgw/rgw_sal_rados.h b/src/rgw/rgw_sal_rados.h index bb6bd4df099d6..bf8a953919582 100644 --- a/src/rgw/rgw_sal_rados.h +++ b/src/rgw/rgw_sal_rados.h @@ -287,12 +287,12 @@ class RadosBucket : public Bucket { virtual int update_container_stats(const DoutPrefixProvider* dpp) override; virtual int check_bucket_shards(const DoutPrefixProvider* dpp) override; virtual int chown(const DoutPrefixProvider* dpp, User* new_user, User* old_user, optional_yield y, const std::string* marker = nullptr) override; - virtual int put_instance_info(const DoutPrefixProvider* dpp, bool exclusive, ceph::real_time mtime) override; + virtual int put_info(const DoutPrefixProvider* dpp, bool exclusive, ceph::real_time mtime) override; virtual int remove_metadata(const DoutPrefixProvider* dpp, RGWObjVersionTracker* objv, optional_yield y) override; virtual bool is_owner(User* user) override; virtual int check_empty(const DoutPrefixProvider* dpp, optional_yield y) override; virtual int check_quota(const DoutPrefixProvider *dpp, RGWQuotaInfo& user_quota, RGWQuotaInfo& bucket_quota, uint64_t obj_size, optional_yield y, bool check_size_only = false) override; - virtual int set_instance_attrs(const DoutPrefixProvider* dpp, Attrs& attrs, optional_yield y) override; + virtual int merge_and_store_attrs(const DoutPrefixProvider* dpp, Attrs& attrs, optional_yield y) override; virtual int try_refresh_info(const DoutPrefixProvider* dpp, ceph::real_time* pmtime) override; virtual int read_usage(const DoutPrefixProvider *dpp, uint64_t start_epoch, uint64_t end_epoch, uint32_t max_entries, bool* is_truncated, RGWUsageIter& usage_iter, -- 2.39.5