librgw: refactor nfsns dirs1 and fix a "bad unlink" case

Fix the error returns from rgw_unlink.  This is sufficient to
catch the errors that arise from operation failure.

We won't notice attempts to remove interior directories--and for
various reasons, that may be problematic (skip for now).

Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>

diff --git a/src/rgw/rgw_file.cc b/src/rgw/rgw_file.cc
index 405ee11a96e13767791cd4018276118cc586d9f7..16817112ba52b3169d1b71077627c42bbfe9e135 100644 (file)
--- a/src/rgw/rgw_file.cc
+++ b/src/rgw/rgw_file.cc
@@ -771,6 +771,9 @@ int rgw_unlink(struct rgw_fs *rgw_fs, struct rgw_file_handle *parent_fh,
     uri += name;
     RGWDeleteBucketRequest req(cct, fs->get_user(), uri);
     rc = rgwlib.get_fe()->execute_req(&req);
+    if (! rc) {
+      rc = req.get_ret();
+    }
   } else {
     /*
      * leaf object
@@ -787,12 +790,15 @@ int rgw_unlink(struct rgw_fs *rgw_fs, struct rgw_file_handle *parent_fh,
       RGWDeleteObjRequest req(cct, fs->get_user(), parent->bucket_name(),
                              oname);
       rc = rgwlib.get_fe()->execute_req(&req);
+      if (! rc) {
+       rc = req.get_ret();
+      }
       /* release */
       (void) rgw_fh_rele(rgw_fs, fh, 0 /* flags */);
     }
   }
 
-  return rc;
+  return -rc;
 }
 
 void dump_buckets(void) {

diff --git a/src/test/librgw_file_nfsns.cc b/src/test/librgw_file_nfsns.cc
index 63f552dc849f9d730d45972447d2684ea2d7857f..5cd1bec5bc70b859e299a5018e363e987cb52f2d 100644 (file)
--- a/src/test/librgw_file_nfsns.cc
+++ b/src/test/librgw_file_nfsns.cc
@@ -109,6 +109,8 @@ namespace {
     dirent_vec() : count(0) {}
   };
 
+  obj_rec dirs1_b{dirs1_bucket_name, nullptr, nullptr, nullptr};
+
   struct {
     int argc;
     char **argv;
@@ -184,45 +186,63 @@ TEST(LibRGW, SETUP_HIER1)
 // create b2
 // create dirs in b2
 // create files in dirs in b2
-TEST(LibRGW, DIRS1) {
+// do getattrs (check type and times)
+TEST(LibRGW, SETUP_DIRS1) {
   if (do_dirs1) {
     int rc;
     struct stat st;
-    obj_rec dirs1_b{dirs1_bucket_name, nullptr, fs->root_fh, nullptr};
-    if (do_create) {
-      rc = rgw_mkdir(fs, dirs1_b.parent_fh, dirs1_b.name.c_str(), 755, &st,
-                   &dirs1_b.fh);
-      ASSERT_EQ(rc, 0);
-    } else {
-      /* TODO: look it up */
-    }
 
-    /* make top-level dirs */
-    int ix;
-    dirs1_vec dirs_vec;
-    for (ix = 0; ix < n_dirs1_dirs; ++ix) {
-      obj_vec ovec;
-      std::string dname{"dir_"};
-      dname += to_string(ix);
-      obj_rec dir{dname, nullptr, dirs1_b.fh, nullptr};
+    dirs1_b.parent_fh = fs->root_fh;
+
+    (void) rgw_lookup(fs, dirs1_b.parent_fh, dirs1_bucket_name.c_str(),
+                     &dirs1_b.fh, RGW_LOOKUP_FLAG_NONE);
+
+    if (! dirs1_b.fh) {
       if (do_create) {
-       rc = rgw_mkdir(fs, dir.parent_fh, dir.name.c_str(), 755, &st, &dir.fh);
+       rc = rgw_mkdir(fs, dirs1_b.parent_fh, dirs1_b.name.c_str(), 755, &st,
+                     &dirs1_b.fh);
        ASSERT_EQ(rc, 0);
-      } else {
-       /* TODO: look it up */
       }
 
-      //dirs_vec.push_back({dir, ovec});
-    }
+      /* make top-level dirs */
+      int ix;
+      dirs1_vec dirs_vec;
+      for (ix = 0; ix < n_dirs1_dirs; ++ix) {
+       obj_vec ovec;
+       std::string dname{"dir_"};
+       dname += to_string(ix);
+       obj_rec dir{dname, nullptr, dirs1_b.fh, nullptr};
+
+       (void) rgw_lookup(fs, dir.parent_fh, dir.name.c_str(), &dir.fh,
+                         RGW_LOOKUP_FLAG_NONE);
+       if (! dir.fh) {
+         if (do_create) {
+           rc = rgw_mkdir(fs, dir.parent_fh, dir.name.c_str(), 755, &st,
+                         &dir.fh);
+           ASSERT_EQ(rc, 0);
+         }
+       }
+       //dirs_vec.push_back({dir, ovec});
+      }
+    } /* dirs1 top-level !exist */
+  }
+}
 
+TEST(LibRGW, BAD_DELETES_DIRS1) {
+  if (do_dirs1) {
+    int rc;
     if (do_delete) {
+      /* try to unlink a non-empty directory (bucket) */
       if (dirs1_b.fh) {
-       rc = rgw_fh_rele(fs, dirs1_b.fh, 0 /* flags */);
-       ASSERT_EQ(rc, 0);
+       // we must eventually release dirs1_b.fh
+       //rc = rgw_fh_rele(fs, dirs1_b.fh, 0 /* flags */);
+       //ASSERT_EQ(rc, 0);
       }
-      rc = rgw_unlink(fs,  dirs1_b.parent_fh, dirs1_b.name.c_str());
-      ASSERT_EQ(rc, 0);
+      rc = rgw_unlink(fs, dirs1_b.parent_fh, dirs1_b.name.c_str());
+      ASSERT_NE(rc, 0);
     }
+    /* try to unlink a non-empty directory (non-bucket) */
+    /* TODO: implement */
   }
 }