From b69757ddddfb52113d5a910546a004ef4e258465 Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Tue, 26 Jul 2022 12:49:55 -0400 Subject: [PATCH] rgw/sal: User::remove_bucket() no longer supports forward_to_master callers of remove_bucket() are now responsible for calling forward_request_to_master() themselves Signed-off-by: Casey Bodley --- src/rgw/driver/daos/rgw_sal_daos.cc | 8 ++------ src/rgw/driver/daos/rgw_sal_daos.h | 1 - src/rgw/driver/motr/rgw_sal_motr.cc | 2 +- src/rgw/driver/motr/rgw_sal_motr.h | 2 +- src/rgw/driver/posix/rgw_sal_posix.cc | 8 +++----- src/rgw/driver/posix/rgw_sal_posix.h | 1 - src/rgw/driver/rados/rgw_bucket.cc | 3 +-- src/rgw/driver/rados/rgw_rest_bucket.cc | 18 ++++++++++++++---- src/rgw/driver/rados/rgw_sal_rados.cc | 17 +---------------- src/rgw/driver/rados/rgw_sal_rados.h | 2 +- src/rgw/driver/rados/rgw_user.cc | 2 +- src/rgw/rgw_op.cc | 4 ++-- src/rgw/rgw_sal.h | 2 +- src/rgw/rgw_sal_dbstore.cc | 2 +- src/rgw/rgw_sal_dbstore.h | 2 +- src/rgw/rgw_sal_filter.cc | 4 +--- src/rgw/rgw_sal_filter.h | 1 - 17 files changed, 31 insertions(+), 48 deletions(-) diff --git a/src/rgw/driver/daos/rgw_sal_daos.cc b/src/rgw/driver/daos/rgw_sal_daos.cc index 73eec5b3e09..c4deba7f11f 100644 --- a/src/rgw/driver/daos/rgw_sal_daos.cc +++ b/src/rgw/driver/daos/rgw_sal_daos.cc @@ -405,13 +405,9 @@ std::unique_ptr DaosBucket::get_encoded_info( } int DaosBucket::remove_bucket(const DoutPrefixProvider* dpp, - bool delete_children, bool forward_to_master, - req_info* req_info, optional_yield y) { + bool delete_children, optional_yield y) { ldpp_dout(dpp, 20) << "DEBUG: remove_bucket, delete_children=" - - << delete_children - - << " forward_to_master=" << forward_to_master << dendl; + << delete_children << dendl; return ds3_bucket_destroy(get_name().c_str(), delete_children, store->ds3, nullptr); diff --git a/src/rgw/driver/daos/rgw_sal_daos.h b/src/rgw/driver/daos/rgw_sal_daos.h index 429c6160488..84b640b8a6e 100644 --- a/src/rgw/driver/daos/rgw_sal_daos.h +++ b/src/rgw/driver/daos/rgw_sal_daos.h @@ -290,7 +290,6 @@ class DaosBucket : public StoreBucket { virtual int list(const DoutPrefixProvider* dpp, ListParams&, int, ListResults&, optional_yield y) override; virtual int remove_bucket(const DoutPrefixProvider* dpp, bool delete_children, - bool forward_to_master, req_info* req_info, optional_yield y) override; virtual int remove_bucket_bypass_gc(int concurrent_max, bool keep_index_consistent, diff --git a/src/rgw/driver/motr/rgw_sal_motr.cc b/src/rgw/driver/motr/rgw_sal_motr.cc index 83c6153a92a..e9377f94536 100644 --- a/src/rgw/driver/motr/rgw_sal_motr.cc +++ b/src/rgw/driver/motr/rgw_sal_motr.cc @@ -556,7 +556,7 @@ int MotrUser::verify_mfa(const std::string& mfa_str, bool* verified, const DoutP return 0; } -int MotrBucket::remove_bucket(const DoutPrefixProvider *dpp, bool delete_children, bool forward_to_master, req_info* req_info, optional_yield y) +int MotrBucket::remove_bucket(const DoutPrefixProvider *dpp, bool delete_children, optional_yield y) { int ret; diff --git a/src/rgw/driver/motr/rgw_sal_motr.h b/src/rgw/driver/motr/rgw_sal_motr.h index ce5fc2b95b1..6e6b9908061 100644 --- a/src/rgw/driver/motr/rgw_sal_motr.h +++ b/src/rgw/driver/motr/rgw_sal_motr.h @@ -352,7 +352,7 @@ class MotrBucket : public StoreBucket { virtual std::unique_ptr get_object(const rgw_obj_key& k) override; virtual int list(const DoutPrefixProvider *dpp, ListParams&, int, ListResults&, optional_yield y) override; - virtual int remove_bucket(const DoutPrefixProvider *dpp, bool delete_children, bool forward_to_master, req_info* req_info, optional_yield y) override; + virtual int remove_bucket(const DoutPrefixProvider *dpp, bool delete_children, optional_yield y) override; virtual int remove_bucket_bypass_gc(int concurrent_max, bool keep_index_consistent, optional_yield y, const diff --git a/src/rgw/driver/posix/rgw_sal_posix.cc b/src/rgw/driver/posix/rgw_sal_posix.cc index 40091d06d0f..b999839bf4e 100644 --- a/src/rgw/driver/posix/rgw_sal_posix.cc +++ b/src/rgw/driver/posix/rgw_sal_posix.cc @@ -950,8 +950,6 @@ int POSIXBucket::merge_and_store_attrs(const DoutPrefixProvider* dpp, int POSIXBucket::remove_bucket(const DoutPrefixProvider* dpp, bool delete_children, - bool forward_to_master, - req_info* req_info, optional_yield y) { return delete_directory(parent_fd, get_fname().c_str(), @@ -963,7 +961,7 @@ int POSIXBucket::remove_bucket_bypass_gc(int concurrent_max, optional_yield y, const DoutPrefixProvider *dpp) { - return remove_bucket(dpp, true, false, nullptr, y); + return remove_bucket(dpp, true, y); } int POSIXBucket::load_bucket(const DoutPrefixProvider* dpp, optional_yield y) @@ -1547,7 +1545,7 @@ int POSIXObject::delete_object(const DoutPrefixProvider* dpp, if (!b->versioned()) { if (shadow) { - ret = shadow->remove_bucket(dpp, true, false, nullptr, y); + ret = shadow->remove_bucket(dpp, true, y); if (ret < 0) { return ret; } @@ -2686,7 +2684,7 @@ int POSIXMultipartUpload::abort(const DoutPrefixProvider *dpp, CephContext *cct, return ret; } - shadow->remove_bucket(dpp, true, false, nullptr, y); + shadow->remove_bucket(dpp, true, y); return 0; } diff --git a/src/rgw/driver/posix/rgw_sal_posix.h b/src/rgw/driver/posix/rgw_sal_posix.h index 739e7ef7a61..3695763c45f 100644 --- a/src/rgw/driver/posix/rgw_sal_posix.h +++ b/src/rgw/driver/posix/rgw_sal_posix.h @@ -196,7 +196,6 @@ public: virtual int merge_and_store_attrs(const DoutPrefixProvider* dpp, Attrs& new_attrs, optional_yield y) override; virtual int remove_bucket(const DoutPrefixProvider* dpp, bool delete_children, - bool forward_to_master, req_info* req_info, optional_yield y) override; virtual int remove_bucket_bypass_gc(int concurrent_max, bool keep_index_consistent, diff --git a/src/rgw/driver/rados/rgw_bucket.cc b/src/rgw/driver/rados/rgw_bucket.cc index fc16316efa4..04fa1538139 100644 --- a/src/rgw/driver/rados/rgw_bucket.cc +++ b/src/rgw/driver/rados/rgw_bucket.cc @@ -1250,8 +1250,7 @@ int RGWBucketAdminOp::remove_bucket(rgw::sal::Driver* driver, RGWBucketAdminOpSt if (bypass_gc) ret = bucket->remove_bucket_bypass_gc(op_state.get_max_aio(), keep_index_consistent, y, dpp); else - ret = bucket->remove_bucket(dpp, op_state.will_delete_children(), - false, nullptr, y); + ret = bucket->remove_bucket(dpp, op_state.will_delete_children(), y); return ret; } diff --git a/src/rgw/driver/rados/rgw_rest_bucket.cc b/src/rgw/driver/rados/rgw_rest_bucket.cc index 69e3f42f15e..9c811efa42d 100644 --- a/src/rgw/driver/rados/rgw_rest_bucket.cc +++ b/src/rgw/driver/rados/rgw_rest_bucket.cc @@ -220,9 +220,19 @@ void RGWOp_Bucket_Remove::execute(optional_yield y) RESTArgs::get_string(s, "bucket", bucket_name, &bucket_name); RESTArgs::get_bool(s, "purge-objects", false, &delete_children); - /* FIXME We're abusing the owner of the bucket to pass the user, so that it can be forwarded to - * the master. This user is actually the OP caller, not the bucket owner. */ - op_ret = driver->get_bucket(s, s->user.get(), string(), bucket_name, &bucket, y); + bufferlist data; + op_ret = driver->forward_request_to_master(s, s->user.get(), nullptr, data, nullptr, s->info, y); + if (op_ret < 0) { + ldpp_dout(this, 0) << "forward_request_to_master returned ret=" << op_ret << dendl; + if (op_ret == -ENOENT) { + /* adjust error, we want to return with NoSuchBucket and not + * NoSuchKey */ + op_ret = -ERR_NO_SUCH_BUCKET; + } + return; + } + + op_ret = driver->get_bucket(s, nullptr, string(), bucket_name, &bucket, y); if (op_ret < 0) { ldpp_dout(this, 0) << "get_bucket returned ret=" << op_ret << dendl; if (op_ret == -ENOENT) { @@ -231,7 +241,7 @@ void RGWOp_Bucket_Remove::execute(optional_yield y) return; } - op_ret = bucket->remove_bucket(s, delete_children, true, &s->info, s->yield); + op_ret = bucket->remove_bucket(s, delete_children, s->yield); } class RGWOp_Set_Bucket_Quota : public RGWRESTOp { diff --git a/src/rgw/driver/rados/rgw_sal_rados.cc b/src/rgw/driver/rados/rgw_sal_rados.cc index f085afb2bee..1de9756fc0a 100644 --- a/src/rgw/driver/rados/rgw_sal_rados.cc +++ b/src/rgw/driver/rados/rgw_sal_rados.cc @@ -404,8 +404,6 @@ RadosBucket::~RadosBucket() {} int RadosBucket::remove_bucket(const DoutPrefixProvider* dpp, bool delete_children, - bool forward_to_master, - req_info* req_info, optional_yield y) { int ret; @@ -488,19 +486,6 @@ int RadosBucket::remove_bucket(const DoutPrefixProvider* dpp, ldpp_dout(dpp, -1) << "ERROR: unable to remove user bucket information" << dendl; } - if (forward_to_master) { - bufferlist in_data; - ret = store->forward_request_to_master(dpp, owner, &ot.read_version, in_data, nullptr, *req_info, y); - if (ret < 0) { - if (ret == -ENOENT) { - /* adjust error, we want to return with NoSuchBucket and not - * NoSuchKey */ - ret = -ERR_NO_SUCH_BUCKET; - } - return ret; - } - } - return ret; } @@ -631,7 +616,7 @@ int RadosBucket::remove_bucket_bypass_gc(int concurrent_max, bool // this function can only be run if caller wanted children to be // deleted, so we can ignore the check for children as any that // remain are detritus from a prior bug - ret = remove_bucket(dpp, true, false, nullptr, y); + ret = remove_bucket(dpp, true, y); if (ret < 0) { ldpp_dout(dpp, -1) << "ERROR: could not remove bucket " << this << dendl; return ret; diff --git a/src/rgw/driver/rados/rgw_sal_rados.h b/src/rgw/driver/rados/rgw_sal_rados.h index e86a0870782..ebf6531f147 100644 --- a/src/rgw/driver/rados/rgw_sal_rados.h +++ b/src/rgw/driver/rados/rgw_sal_rados.h @@ -537,7 +537,7 @@ class RadosBucket : public StoreBucket { virtual ~RadosBucket(); virtual std::unique_ptr get_object(const rgw_obj_key& k) override; virtual int list(const DoutPrefixProvider* dpp, ListParams&, int, ListResults&, optional_yield y) override; - virtual int remove_bucket(const DoutPrefixProvider* dpp, bool delete_children, bool forward_to_master, req_info* req_info, optional_yield y) override; + virtual int remove_bucket(const DoutPrefixProvider* dpp, bool delete_children, optional_yield y) override; virtual int remove_bucket_bypass_gc(int concurrent_max, bool keep_index_consistent, optional_yield y, const diff --git a/src/rgw/driver/rados/rgw_user.cc b/src/rgw/driver/rados/rgw_user.cc index 469fe6fe68f..f81254bcfb0 100644 --- a/src/rgw/driver/rados/rgw_user.cc +++ b/src/rgw/driver/rados/rgw_user.cc @@ -1791,7 +1791,7 @@ int RGWUser::execute_remove(const DoutPrefixProvider *dpp, RGWUserAdminOpState& return ret; } - ret = bucket->remove_bucket(dpp, true, false, nullptr, y); + ret = bucket->remove_bucket(dpp, true, y); if (ret < 0) { set_err_msg(err_msg, "unable to delete user data"); return ret; diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc index 32416457213..a830ff726af 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -3594,7 +3594,7 @@ void RGWDeleteBucket::execute(optional_yield y) // do nothing; it will already have been logged } - op_ret = s->bucket->remove_bucket(this, false, false, nullptr, y); + op_ret = s->bucket->remove_bucket(this, false, y); if (op_ret < 0 && op_ret == -ECANCELED) { // lost a race, either with mdlog sync or another delete bucket operation. // in either case, we've already called ctl.bucket->unlink_bucket() @@ -7351,7 +7351,7 @@ bool RGWBulkDelete::Deleter::delete_single(const acct_path_t& path, optional_yie goto delop_fail; } } - ret = bucket->remove_bucket(dpp, false, false, nullptr, s->yield); + ret = bucket->remove_bucket(dpp, false, s->yield); if (ret < 0) { goto delop_fail; } diff --git a/src/rgw/rgw_sal.h b/src/rgw/rgw_sal.h index e7a3dfe0aa5..73122e06b4b 100644 --- a/src/rgw/rgw_sal.h +++ b/src/rgw/rgw_sal.h @@ -660,7 +660,7 @@ class Bucket { /** Set the cached attributes on this bucket */ virtual int set_attrs(Attrs a) = 0; /** Remove this bucket from the backing store */ - virtual int remove_bucket(const DoutPrefixProvider* dpp, bool delete_children, bool forward_to_master, req_info* req_info, optional_yield y) = 0; + virtual int remove_bucket(const DoutPrefixProvider* dpp, bool delete_children, optional_yield y) = 0; /** Remove this bucket, bypassing garbage collection. May be removed */ virtual int remove_bucket_bypass_gc(int concurrent_max, bool keep_index_consistent, diff --git a/src/rgw/rgw_sal_dbstore.cc b/src/rgw/rgw_sal_dbstore.cc index 36d76cc12e1..97da47eb6d3 100644 --- a/src/rgw/rgw_sal_dbstore.cc +++ b/src/rgw/rgw_sal_dbstore.cc @@ -236,7 +236,7 @@ namespace rgw::sal { return 0; } - int DBBucket::remove_bucket(const DoutPrefixProvider *dpp, bool delete_children, bool forward_to_master, req_info* req_info, optional_yield y) + int DBBucket::remove_bucket(const DoutPrefixProvider *dpp, bool delete_children, optional_yield y) { int ret; diff --git a/src/rgw/rgw_sal_dbstore.h b/src/rgw/rgw_sal_dbstore.h index 0c75f4b98cb..68fe4c44319 100644 --- a/src/rgw/rgw_sal_dbstore.h +++ b/src/rgw/rgw_sal_dbstore.h @@ -171,7 +171,7 @@ protected: virtual std::unique_ptr get_object(const rgw_obj_key& k) override; virtual int list(const DoutPrefixProvider *dpp, ListParams&, int, ListResults&, optional_yield y) override; - virtual int remove_bucket(const DoutPrefixProvider *dpp, bool delete_children, bool forward_to_master, req_info* req_info, optional_yield y) override; + virtual int remove_bucket(const DoutPrefixProvider *dpp, bool delete_children, optional_yield y) override; virtual int remove_bucket_bypass_gc(int concurrent_max, bool keep_index_consistent, optional_yield y, const diff --git a/src/rgw/rgw_sal_filter.cc b/src/rgw/rgw_sal_filter.cc index dbf688a22ab..fb9e7aa1a28 100644 --- a/src/rgw/rgw_sal_filter.cc +++ b/src/rgw/rgw_sal_filter.cc @@ -649,11 +649,9 @@ int FilterBucket::list(const DoutPrefixProvider* dpp, ListParams& params, int ma int FilterBucket::remove_bucket(const DoutPrefixProvider* dpp, bool delete_children, - bool forward_to_master, - req_info* req_info, optional_yield y) { - return next->remove_bucket(dpp, delete_children, forward_to_master, req_info, y); + return next->remove_bucket(dpp, delete_children, y); } int FilterBucket::remove_bucket_bypass_gc(int concurrent_max, diff --git a/src/rgw/rgw_sal_filter.h b/src/rgw/rgw_sal_filter.h index dcc03df9519..81452f22cd7 100644 --- a/src/rgw/rgw_sal_filter.h +++ b/src/rgw/rgw_sal_filter.h @@ -413,7 +413,6 @@ public: virtual Attrs& get_attrs(void) override { return next->get_attrs(); } virtual int set_attrs(Attrs a) override { return next->set_attrs(a); } virtual int remove_bucket(const DoutPrefixProvider* dpp, bool delete_children, - bool forward_to_master, req_info* req_info, optional_yield y) override; virtual int remove_bucket_bypass_gc(int concurrent_max, bool keep_index_consistent, -- 2.39.5