rgw/dedup: fix negative errno codes in filter and radosgw-admin

Signed-off-by: benhanokh <gbenhano@redhat.com>

diff --git a/src/rgw/driver/rados/rgw_dedup_filter.cc b/src/rgw/driver/rados/rgw_dedup_filter.cc
index c3b62f6f4af1aa51dfea6ab14a8330060cb74d61..f7e9dd669b70cdca45ef81ef4656bbcf6972b404 100644 (file)
--- a/src/rgw/driver/rados/rgw_dedup_filter.cc
+++ b/src/rgw/driver/rados/rgw_dedup_filter.cc
@@ -21,6 +21,7 @@
 #include <fstream>
 #include <iostream>
 #include <string>
+#include <algorithm>
 
 #define dout_subsys ceph_subsys_rgw_dedup
 
@@ -83,7 +84,7 @@ namespace rgw::dedup {
     std::ifstream f(path);
     if (!f.is_open()) {
       ldpp_dout(dpp, 1) << __func__ << ":: failed to open filter file: " << path << dendl;
-      return ENOENT;
+      return -ENOENT;
     }
 
     std::string line;
@@ -115,7 +116,7 @@ namespace rgw::dedup {
         else {
           ldpp_dout(dpp, 1) << __func__ << "::" << path << "::" << line_num
                             << "::invalid name '" << name << "'" << dendl;
-          return EINVAL;
+          return -EINVAL;
         }
       }
     }
@@ -135,14 +136,14 @@ namespace rgw::dedup {
       ldpp_dout(dpp, 1) << __func__
                         << ":: --allow-bucket-list and --deny-bucket-list are mutually exclusive"
                         << dendl;
-      d_errcode = EINVAL;
+      d_errcode = -EINVAL;
       return;
     }
     if (!allow_sc_file.empty() && !deny_sc_file.empty()) {
       ldpp_dout(dpp, 1) << __func__
                         << ":: --allow-storage-class-list and --deny-storage-class-list are mutually exclusive"
                         << dendl;
-      d_errcode = EINVAL;
+      d_errcode = -EINVAL;
       return;
     }
 

diff --git a/src/rgw/driver/rados/rgw_dedup_filter.h b/src/rgw/driver/rados/rgw_dedup_filter.h
index 8bda372e20b179ae0cb2338a01c88d2df1eaa068..aa11b467bf8629d8c3dfe7935092ebfbe0a518ea 100644 (file)
--- a/src/rgw/driver/rados/rgw_dedup_filter.h
+++ b/src/rgw/driver/rados/rgw_dedup_filter.h
@@ -18,6 +18,7 @@
 #include "include/encoding.h"
 #include <string>
 #include <unordered_set>
+#include <vector>
 
 namespace rgw::dedup {
 
@@ -43,7 +44,7 @@ namespace rgw::dedup {
                    const std::string& deny_sc_file,
                    const DoutPrefixProvider* dpp);
 
-    // Returns 0 on success, errno if construction failed.
+    // Returns 0 on success, negative errno if construction failed.
     int errcode() const { return d_errcode; }
 
     // Returns true if any filter dimension is active.
@@ -67,7 +68,7 @@ namespace rgw::dedup {
 
   private:
     // Read filter file: one name per line, '#' starts a comment, whitespace trimmed
-    // On success returns 0 and populates name_set; on error returns errno code.
+    // On success returns 0 and populates name_set; on error returns negative errno.
     static int read_filter_file(const std::string& path,
                                 std::unordered_set<std::string>& name_set,
                                 int (*validator)(const std::string&),

diff --git a/src/rgw/radosgw-admin/radosgw-admin.cc b/src/rgw/radosgw-admin/radosgw-admin.cc
index bb1170329f801aa466e1a21d7ee1cebd17c088cf..2f2cbe9dc78730ac8fb5b54e9c4550a5f03dead0 100644 (file)
--- a/src/rgw/radosgw-admin/radosgw-admin.cc
+++ b/src/rgw/radosgw-admin/radosgw-admin.cc
@@ -9396,7 +9396,7 @@ next:
       else {
        cerr << "ERROR: Failed reading stat counters" << std::endl;
       }
-      return ret;
+      return -ret;
     }
 
     if (opt_cmd == OPT::DEDUP_THROTTLE) {
@@ -9407,7 +9407,7 @@ next:
 
       if (throttle_stat) {
        encode(throttle_msg, urgent_msg_bl);
-       return cluster::dedup_control_bl(store, dpp(), urgent_msg, urgent_msg_bl);
+       return -cluster::dedup_control_bl(store, dpp(), urgent_msg, urgent_msg_bl);
       }
 
       if (unlikely(!have_max_bucket_index_ops && !have_max_metadata_ops)) {
@@ -9428,7 +9428,7 @@ next:
       }
 
       encode(throttle_msg, urgent_msg_bl);
-      return cluster::dedup_control_bl(store, dpp(), urgent_msg, urgent_msg_bl);
+      return -cluster::dedup_control_bl(store, dpp(), urgent_msg, urgent_msg_bl);
     }
 
     if (opt_cmd == OPT::DEDUP_ABORT  ||
@@ -9444,7 +9444,7 @@ next:
       else {
        urgent_msg = URGENT_MSG_RESUME;
       }
-      return cluster::dedup_control(store, dpp(), urgent_msg);
+      return -cluster::dedup_control(store, dpp(), urgent_msg);
     }
 
     if (opt_cmd == OPT::DEDUP_EXEC || opt_cmd == OPT::DEDUP_ESTIMATE) {
@@ -9473,12 +9473,14 @@ next:
       int filter_err = dedup_filter.errcode();
       if (filter_err != 0) {
        cerr << "ERROR: failed to build dedup filter: "
-             << cpp_strerror(filter_err) << std::endl;
-       return filter_err;
+             << cpp_strerror(-filter_err) << std::endl;
+       return -filter_err;
       }
 
       int ret = cluster::dedup_restart_scan(store, dedup_type, dpp(),
                                            dedup_filter.is_active() ? &dedup_filter : nullptr);
+      // reverse negative errno codes
+      ret = -ret;
       if (ret == 0) {
        std::cout << "Dedup was restarted successfully" << std::endl;
       }

diff --git a/src/test/rgw/dedup/test_dedup.py b/src/test/rgw/dedup/test_dedup.py
index c8ffa50ddb6616376eb8dd6ae19b83d9fac281fc..84f15516e1b147c8b677f3c7b32f6941740714fd 100644 (file)
--- a/src/test/rgw/dedup/test_dedup.py
+++ b/src/test/rgw/dedup/test_dedup.py
@@ -2307,9 +2307,10 @@ def test_copy_after_dedup():
     # create files in range [8MB, 32MB] aligned on RADOS_OBJ_SIZE
     gen_files_in_range(files, num_files, 8*MB, 32*MB)
 
-    bucket_cp= gen_bucket_name()
+    bucket_cp=gen_bucket_name()
     bucket_names=[]
     conns=[]
+    conn=None
     try:
         conn = get_single_connection()
         conn.create_bucket(Bucket=bucket_cp)
@@ -2359,9 +2360,15 @@ def test_copy_after_dedup():
 
     finally:
         # cleanup must be executed even after a failure
-        delete_bucket_with_all_objects(bucket_cp, conn)
-        if len(bucket_names) > 0:
-            cleanup_all_buckets(bucket_names, conns)
+        if conn:
+            delete_bucket_with_all_objects(bucket_cp, conn)
+            if len(bucket_names) > 0:
+                cleanup_all_buckets(bucket_names, conns)
+
+            result = admin(['gc', 'process', '--include-all'])
+            assert result[1] == 0
+        else:
+            cleanup_local()
 
 #-------------------------------------------------------------------------------
 @pytest.mark.basic_test