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 <ivancich@redhat.com>

diff --git a/src/rgw/rgw_bucket.cc b/src/rgw/rgw_bucket.cc
index 4aaa4e206a0e645c135d422c39b1ec3eac4fdde7..07f89d94986d832592cb698b44e697e38c9ae1ad 100644 (file)
--- 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 b2354f3140f33b4ebe22543f0e92f5af9c4c7a53..91b337966a0f42f64a870123e22091192e4245f1 100644 (file)
--- 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 8dcb0aac5f7b4970d7d3532696e35d6433df2c1e..8df99378ea70d58a55505d4ecd6a2d4ef5c210d4 100644 (file)
--- 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 6bad56ceb32e2f191257bda61533fdffcd6259e2..b67b6fd578f9bac77e987b013908ed401c0d941f 100644 (file)
--- 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<std::unique_ptr<MultipartUpload>>& uploads,
                                std::map<std::string, bool> *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 bc1934c9df746d5433f9896ebf1a0f50ea5bf333..9808a147ec13ebbf1b7e04dba0e2513995e0d6e8 100644 (file)
--- 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 bb4748520434399fc9af2d1a07b8cde611e30477..324a27820109f5fad5880ac3e6d38060a3f3dfd4 100644 (file)
--- a/src/rgw/rgw_sal_dbstore.h
+++ b/src/rgw/rgw_sal_dbstore.h
@@ -149,7 +149,7 @@ protected:
 
       virtual std::unique_ptr<Object> 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<std::unique_ptr<MultipartUpload>>& uploads,
                                map<string, bool> *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 1bf8417d186a5d4acf124dbe2bbba4652e7d0084..21d46511f36d1ce8b3cad8f8428ed6ac43176fa9 100644 (file)
--- 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 bc19927a4df1cc1cc3558d5d411f91c7690d0015..887e9afc466da5e98cb6ab0e9404c28b8b00794b 100644 (file)
--- 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<Object> 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<std::unique_ptr<MultipartUpload>>& uploads,
                                std::map<std::string, bool> *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 e231f33624a5915adb874f2a9f3687a03fa19b70..b53b048e3f8a8aba8e98aadbece1c36af12cb34f 100644 (file)
--- 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;