From 5156e4e7ce307f8ac6ee6dc36f7e546c93d17d55 Mon Sep 17 00:00:00 2001 From: "J. Eric Ivancich" Date: Fri, 12 Nov 2021 17:24:35 -0500 Subject: [PATCH] rgw: remove prefix & delim params for bucket removal & mp upload abort The calls to remove a bucket had parameters to specify a prefix and delimiter, which does not make sense. This was precipitated due to some existing Swift protocol logic, but buckets are removed irrespective of prefix and delimiter. So the functions and calls are adjusted to remove those parameters. Additionally, those same parameters were removed for aborting incomplete multipart uploads. Additionally a bug is fixed in which during bucket removal, multipart uploads were only removed if the prefix was non-empty. Signed-off-by: J. Eric Ivancich --- src/rgw/rgw_bucket.cc | 2 +- src/rgw/rgw_op.cc | 21 +++------------------ src/rgw/rgw_rest_bucket.cc | 2 +- src/rgw/rgw_sal.h | 9 ++++----- src/rgw/rgw_sal_dbstore.cc | 7 +++---- src/rgw/rgw_sal_dbstore.h | 7 +++---- src/rgw/rgw_sal_rados.cc | 35 +++++++++++++++-------------------- src/rgw/rgw_sal_rados.h | 7 +++---- src/rgw/rgw_user.cc | 3 +-- 9 files changed, 34 insertions(+), 59 deletions(-) diff --git a/src/rgw/rgw_bucket.cc b/src/rgw/rgw_bucket.cc index 4aaa4e206a0e6..07f89d94986d8 100644 --- a/src/rgw/rgw_bucket.cc +++ b/src/rgw/rgw_bucket.cc @@ -1020,7 +1020,7 @@ int RGWBucketAdminOp::remove_bucket(rgw::sal::Store* store, RGWBucketAdminOpStat 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(), string(), string(), + ret = bucket->remove_bucket(dpp, op_state.will_delete_children(), false, nullptr, y); return ret; diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc index b2354f3140f33..91b337966a0f4 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -3415,27 +3415,13 @@ void RGWDeleteBucket::execute(optional_yield y) return; } - string prefix, delimiter; - - if (s->prot_flags & RGW_REST_SWIFT) { - string path_args; - path_args = s->info.args.get("path"); - if (!path_args.empty()) { - if (!delimiter.empty() || !prefix.empty()) { - op_ret = -EINVAL; - return; - } - prefix = path_args; - delimiter="/"; - } - } - - op_ret = s->bucket->remove_bucket(this, false, prefix, delimiter, false, nullptr, y); + op_ret = s->bucket->remove_bucket(this, false, false, nullptr, 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() op_ret = 0; } + return; } @@ -6987,7 +6973,7 @@ bool RGWBulkDelete::Deleter::delete_single(const acct_path_t& path, optional_yie goto delop_fail; } } else { - ret = bucket->remove_bucket(dpp, false, string(), string(), true, &s->info, s->yield); + ret = bucket->remove_bucket(dpp, false, true, &s->info, s->yield); if (ret < 0) { goto delop_fail; } @@ -6996,7 +6982,6 @@ bool RGWBulkDelete::Deleter::delete_single(const acct_path_t& path, optional_yie num_deleted++; return true; - binfo_fail: if (-ENOENT == ret) { ldpp_dout(dpp, 20) << "cannot find bucket = " << path.bucket_name << dendl; diff --git a/src/rgw/rgw_rest_bucket.cc b/src/rgw/rgw_rest_bucket.cc index 8dcb0aac5f7b4..8df99378ea70d 100644 --- a/src/rgw/rgw_rest_bucket.cc +++ b/src/rgw/rgw_rest_bucket.cc @@ -228,7 +228,7 @@ void RGWOp_Bucket_Remove::execute(optional_yield y) return; } - op_ret = bucket->remove_bucket(s, delete_children, string(), string(), true, &s->info, s->yield); + op_ret = bucket->remove_bucket(s, delete_children, true, &s->info, s->yield); } class RGWOp_Set_Bucket_Quota : public RGWRESTOp { diff --git a/src/rgw/rgw_sal.h b/src/rgw/rgw_sal.h index 6bad56ceb32e2..b67b6fd578f9b 100644 --- a/src/rgw/rgw_sal.h +++ b/src/rgw/rgw_sal.h @@ -601,7 +601,7 @@ class Bucket { /** Set the cached attributes on this bucket */ virtual int set_attrs(Attrs a) { attrs = a; return 0; } /** Remove this bucket from the backing store */ - virtual int remove_bucket(const DoutPrefixProvider* dpp, bool delete_children, std::string prefix, std::string delimiter, bool forward_to_master, req_info* req_info, optional_yield y) = 0; + virtual int remove_bucket(const DoutPrefixProvider* dpp, bool delete_children, bool forward_to_master, req_info* req_info, 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, @@ -719,10 +719,9 @@ class Bucket { std::vector>& uploads, std::map *common_prefixes, bool *is_truncated) = 0; - /** Abort multipart uploads matching the prefix and delimiter */ - virtual int abort_multiparts(const DoutPrefixProvider *dpp, - CephContext *cct, - std::string& prefix, std::string& delim) = 0; + /** Abort multipart uploads in a bucket */ + virtual int abort_multiparts(const DoutPrefixProvider* dpp, + CephContext* cct) = 0; /* dang - This is temporary, until the API is completed */ rgw_bucket& get_key() { return info.bucket; } diff --git a/src/rgw/rgw_sal_dbstore.cc b/src/rgw/rgw_sal_dbstore.cc index bc1934c9df746..9808a147ec13e 100644 --- a/src/rgw/rgw_sal_dbstore.cc +++ b/src/rgw/rgw_sal_dbstore.cc @@ -218,7 +218,7 @@ namespace rgw::sal { return ret; } - int DBBucket::remove_bucket(const DoutPrefixProvider *dpp, bool delete_children, std::string prefix, std::string delimiter, bool forward_to_master, req_info* req_info, optional_yield y) + int DBBucket::remove_bucket(const DoutPrefixProvider *dpp, bool delete_children, bool forward_to_master, req_info* req_info, optional_yield y) { int ret; @@ -460,9 +460,8 @@ namespace rgw::sal { return 0; } - int DBBucket::abort_multiparts(const DoutPrefixProvider *dpp, - CephContext *cct, - string& prefix, string& delim) { + int DBBucket::abort_multiparts(const DoutPrefixProvider* dpp, + CephContext* cct) { return 0; } diff --git a/src/rgw/rgw_sal_dbstore.h b/src/rgw/rgw_sal_dbstore.h index bb47485204343..324a27820109f 100644 --- a/src/rgw/rgw_sal_dbstore.h +++ b/src/rgw/rgw_sal_dbstore.h @@ -149,7 +149,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, std::string prefix, std::string delimiter, bool forward_to_master, req_info* req_info, 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, optional_yield y, const @@ -196,9 +196,8 @@ protected: vector>& uploads, map *common_prefixes, bool *is_truncated) override; - virtual int abort_multiparts(const DoutPrefixProvider *dpp, - CephContext *cct, - string& prefix, string& delim) override; + virtual int abort_multiparts(const DoutPrefixProvider* dpp, + CephContext* cct) override; friend class DBStore; }; diff --git a/src/rgw/rgw_sal_rados.cc b/src/rgw/rgw_sal_rados.cc index 1bf8417d186a5..21d46511f36d1 100644 --- a/src/rgw/rgw_sal_rados.cc +++ b/src/rgw/rgw_sal_rados.cc @@ -360,10 +360,9 @@ RadosBucket::~RadosBucket() {} int RadosBucket::remove_bucket(const DoutPrefixProvider* dpp, bool delete_children, - std::string prefix, - std::string delimiter, bool forward_to_master, - req_info* req_info, optional_yield y) + req_info* req_info, + optional_yield y) { int ret; @@ -401,12 +400,9 @@ int RadosBucket::remove_bucket(const DoutPrefixProvider* dpp, } } while(results.is_truncated); - /* If there's a prefix, then we are aborting multiparts as well */ - if (!prefix.empty()) { - ret = abort_multiparts(dpp, store->ctx(), prefix, delimiter); - if (ret < 0) { - return ret; - } + ret = abort_multiparts(dpp, store->ctx()); + if (ret < 0) { + return ret; } ret = store->ctl()->bucket->sync_user_stats(dpp, info.owner, info, y); @@ -476,9 +472,7 @@ int RadosBucket::remove_bucket_bypass_gc(int concurrent_max, bool if (ret < 0) return ret; - string prefix, delimiter; - - ret = abort_multiparts(dpp, cct, prefix, delimiter); + ret = abort_multiparts(dpp, cct); if (ret < 0) { return ret; } @@ -579,7 +573,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, std::string(), std::string(), false, nullptr, y); + ret = remove_bucket(dpp, true, false, nullptr, y); if (ret < 0) { ldpp_dout(dpp, -1) << "ERROR: could not remove bucket " << this << dendl; return ret; @@ -917,9 +911,8 @@ int RadosBucket::list_multiparts(const DoutPrefixProvider *dpp, return 0; } -int RadosBucket::abort_multiparts(const DoutPrefixProvider *dpp, - CephContext *cct, - string& prefix, string& delim) +int RadosBucket::abort_multiparts(const DoutPrefixProvider* dpp, + CephContext* cct) { constexpr int max = 1000; int ret, num_deleted = 0; @@ -928,14 +921,16 @@ int RadosBucket::abort_multiparts(const DoutPrefixProvider *dpp, string marker; bool is_truncated; + const std::string empty_delim; + const std::string empty_prefix; + do { - ret = list_multiparts(dpp, prefix, marker, delim, - max, uploads, nullptr, &is_truncated); + ret = list_multiparts(dpp, empty_prefix, marker, empty_delim, + max, uploads, nullptr, &is_truncated); if (ret < 0) { ldpp_dout(dpp, 0) << __func__ << " ERROR : calling list_bucket_multiparts; ret=" << ret << - "; bucket=\"" << this << "\"; prefix=\"" << - prefix << "\"; delim=\"" << delim << "\"" << dendl; + "; bucket=\"" << this << "\"" << dendl; return ret; } ldpp_dout(dpp, 20) << __func__ << diff --git a/src/rgw/rgw_sal_rados.h b/src/rgw/rgw_sal_rados.h index bc19927a4df1c..887e9afc466da 100644 --- a/src/rgw/rgw_sal_rados.h +++ b/src/rgw/rgw_sal_rados.h @@ -282,7 +282,7 @@ class RadosBucket : public Bucket { 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, std::string prefix, std::string delimiter, bool forward_to_master, req_info* req_info, 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, optional_yield y, const @@ -330,9 +330,8 @@ class RadosBucket : public Bucket { std::vector>& uploads, std::map *common_prefixes, bool *is_truncated) override; - virtual int abort_multiparts(const DoutPrefixProvider *dpp, - CephContext *cct, - std::string& prefix, std::string& delim) override; + virtual int abort_multiparts(const DoutPrefixProvider* dpp, + CephContext* cct) override; private: int link(const DoutPrefixProvider* dpp, User* new_user, optional_yield y, bool update_entrypoint = true, RGWObjVersionTracker* objv = nullptr); diff --git a/src/rgw/rgw_user.cc b/src/rgw/rgw_user.cc index e231f33624a59..b53b048e3f8a8 100644 --- a/src/rgw/rgw_user.cc +++ b/src/rgw/rgw_user.cc @@ -1939,9 +1939,8 @@ int RGWUser::execute_remove(const DoutPrefixProvider *dpp, RGWUserAdminOpState& return -EEXIST; // change to code that maps to 409: conflict } - std::string prefix, delimiter; for (auto it = m.begin(); it != m.end(); ++it) { - ret = it->second->remove_bucket(dpp, true, prefix, delimiter, false, nullptr, y); + ret = it->second->remove_bucket(dpp, true, false, nullptr, y); if (ret < 0) { set_err_msg(err_msg, "unable to delete user data"); return ret; -- 2.39.5