From b9223477ace38eef8a5efda33cd98e15e965bb55 Mon Sep 17 00:00:00 2001 From: "J. Eric Ivancich" Date: Tue, 16 Nov 2021 18:06:58 -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 (cherry-picked from 5156e4e7ce307f8ac6ee6dc36f7e546c93d17d55) Conflicts: src/rgw/rgw_sal_rados.cc src/rgw/rgw_sal.h src/rgw/rgw_sal_rados.h - Alterations due to Zipper 7 code refactoring src/rgw/rgw_sal_dbstore.cc src/rgw/rgw_sal_dbstore.h - Did not exist before Zipper 7 code refactoring --- src/rgw/rgw_bucket.cc | 6 ++---- src/rgw/rgw_multi.cc | 12 ++++++------ src/rgw/rgw_multi.h | 4 +--- src/rgw/rgw_op.cc | 19 ++----------------- src/rgw/rgw_rest_bucket.cc | 2 +- src/rgw/rgw_sal.h | 2 +- src/rgw/rgw_sal_rados.cc | 11 +++-------- src/rgw/rgw_sal_rados.h | 2 +- src/rgw/rgw_user.cc | 3 +-- 9 files changed, 18 insertions(+), 43 deletions(-) diff --git a/src/rgw/rgw_bucket.cc b/src/rgw/rgw_bucket.cc index 939e5e95029d2..725df23a1400a 100644 --- a/src/rgw/rgw_bucket.cc +++ b/src/rgw/rgw_bucket.cc @@ -385,9 +385,7 @@ int rgw_remove_bucket_bypass_gc(rgw::sal::RGWRadosStore *store, rgw_bucket& buck if (ret < 0) return ret; - string prefix, delimiter; - - ret = abort_bucket_multiparts(dpp, store, cct, info, prefix, delimiter); + ret = abort_bucket_multiparts(dpp, store, cct, info); if (ret < 0) { return ret; } @@ -1279,7 +1277,7 @@ int RGWBucketAdminOp::remove_bucket(rgw::sal::RGWRadosStore *store, RGWBucketAdm if (bypass_gc) ret = rgw_remove_bucket_bypass_gc(store, bucket->get_key(), 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_multi.cc b/src/rgw/rgw_multi.cc index eeccacb6a2995..f035d1bc21477 100644 --- a/src/rgw/rgw_multi.cc +++ b/src/rgw/rgw_multi.cc @@ -319,9 +319,7 @@ int list_bucket_multiparts(const DoutPrefixProvider* dpp, int abort_bucket_multiparts(const DoutPrefixProvider* dpp, rgw::sal::RGWRadosStore* store, CephContext* cct, - RGWBucketInfo& bucket_info, - const std::string& prefix, - const std::string& delim) + RGWBucketInfo& bucket_info) { constexpr int max = 1000; int ret, num_deleted = 0; @@ -330,14 +328,16 @@ int abort_bucket_multiparts(const DoutPrefixProvider* dpp, string marker; bool is_truncated; + const std::string empty_delim; + const std::string empty_prefix; + do { - ret = list_bucket_multiparts(dpp, store, bucket_info, prefix, marker, delim, + ret = list_bucket_multiparts(dpp, store, bucket_info, empty_prefix, marker, empty_delim, max, &objs, nullptr, &is_truncated); if (ret < 0) { ldpp_dout(dpp, 0) << __func__ << " ERROR : calling list_bucket_multiparts; ret=" << ret << - "; bucket=\"" << bucket_info.bucket << "\"; prefix=\"" << - prefix << "\"; delim=\"" << delim << "\"" << dendl; + "; bucket=\"" << bucket_info.bucket << "\"" << dendl; return ret; } ldpp_dout(dpp, 20) << __func__ << diff --git a/src/rgw/rgw_multi.h b/src/rgw/rgw_multi.h index 9a3321542ce46..31069417e9169 100644 --- a/src/rgw/rgw_multi.h +++ b/src/rgw/rgw_multi.h @@ -143,7 +143,5 @@ extern int list_bucket_multiparts(const DoutPrefixProvider* dpp, extern int abort_bucket_multiparts(const DoutPrefixProvider* dpp, rgw::sal::RGWRadosStore* store, CephContext* cct, - RGWBucketInfo& bucket_info, - const std::string& prefix, - const std::string& delim); + RGWBucketInfo& bucket_info); #endif diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc index 770f780f5c8ce..02ae8a93fd046 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -3351,22 +3351,7 @@ 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() @@ -7001,7 +6986,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; } diff --git a/src/rgw/rgw_rest_bucket.cc b/src/rgw/rgw_rest_bucket.cc index d0d07174404e5..52520269e561b 100644 --- a/src/rgw/rgw_rest_bucket.cc +++ b/src/rgw/rgw_rest_bucket.cc @@ -227,7 +227,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 156bc85226884..41da8bc9d854e 100644 --- a/src/rgw/rgw_sal.h +++ b/src/rgw/rgw_sal.h @@ -232,7 +232,7 @@ class RGWBucket { virtual RGWObject* create_object(const rgw_obj_key& key /* Attributes */) = 0; virtual RGWAttrs& get_attrs(void) { return attrs; } virtual int set_attrs(RGWAttrs a) { attrs = a; return 0; } - 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; virtual RGWAccessControlPolicy& get_acl(void) = 0; virtual int set_acl(const DoutPrefixProvider *dpp, RGWAccessControlPolicy& acl, optional_yield y) = 0; virtual int get_bucket_info(const DoutPrefixProvider *dpp, optional_yield y) = 0; diff --git a/src/rgw/rgw_sal_rados.cc b/src/rgw/rgw_sal_rados.cc index added3e6a3299..28f1cef9ffefd 100644 --- a/src/rgw/rgw_sal_rados.cc +++ b/src/rgw/rgw_sal_rados.cc @@ -88,8 +88,6 @@ RGWObject *RGWRadosBucket::create_object(const rgw_obj_key &key) int RGWRadosBucket::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) { @@ -129,12 +127,9 @@ int RGWRadosBucket::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_bucket_multiparts(dpp, store, store->ctx(), info, prefix, delimiter); - if (ret < 0) { - return ret; - } + ret = abort_bucket_multiparts(dpp, store, store->ctx(), info); + if (ret < 0) { + return ret; } ret = store->ctl()->bucket->sync_user_stats(dpp, info.owner, info, y); diff --git a/src/rgw/rgw_sal_rados.h b/src/rgw/rgw_sal_rados.h index 79596c6f868dc..8000756370e4d 100644 --- a/src/rgw/rgw_sal_rados.h +++ b/src/rgw/rgw_sal_rados.h @@ -230,7 +230,7 @@ class RGWRadosBucket : public RGWBucket { RGWBucketList* list(void) { return new RGWBucketList(); } virtual int list(const DoutPrefixProvider *dpp, ListParams&, int, ListResults&, optional_yield y) override; RGWObject* create_object(const rgw_obj_key& key /* Attributes */) 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; RGWAccessControlPolicy& get_acl(void) { return acls; } virtual int set_acl(const DoutPrefixProvider *dpp, RGWAccessControlPolicy& acl, optional_yield y) override; virtual int get_bucket_info(const DoutPrefixProvider *dpp, optional_yield y) override; diff --git a/src/rgw/rgw_user.cc b/src/rgw/rgw_user.cc index f9499bfa79ecc..ad8cfc88d134e 100644 --- a/src/rgw/rgw_user.cc +++ b/src/rgw/rgw_user.cc @@ -1938,9 +1938,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