client: wait rename to finish

In rare case during the rename if another thread tries to lookup the
dst dentry, it may get an inconsistent result that both src dentry
and dst dentry will link to the same inode at the same time.

Will wait the rename to finish and try it again.

Fixes: https://tracker.ceph.com/issues/49912
Signed-off-by: Xiubo Li <xiubli@redhat.com>
(cherry picked from commit 11fb16df06ad61e228c17feedf00dfb85f0477d5)

diff --git a/src/client/Client.cc b/src/client/Client.cc
index 8613f19ed94a2ec27eca87b72e3b3e62960c9e93..c5173e21a65c8c5abdbaac3d42b0fdb320e011b9 100644 (file)
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -1209,6 +1209,11 @@ Dentry *Client::insert_dentry_inode(Dir *dir, const string& dname, LeaseStat *dl
     Inode *diri = dir->parent_inode;
     clear_dir_complete_and_ordered(diri, false);
     dn = link(dir, dname, in, dn);
+
+    if (old_dentry) {
+      dn->is_renaming = false;
+      signal_cond_list(waiting_for_rename);
+    }
   }
 
   update_dentry_lease(dn, dlease, from, session);
@@ -7052,7 +7057,8 @@ bool Client::_dentry_valid(const Dentry *dn)
 }
 
 int Client::_lookup(Inode *dir, const string& dname, int mask, InodeRef *target,
-                   const UserPerm& perms, std::string* alternate_name)
+                    const UserPerm& perms, std::string* alternate_name,
+                    bool is_rename)
 {
   int r = 0;
   Dentry *dn = NULL;
@@ -7131,6 +7137,19 @@ relookup:
     } else {
       ldout(cct, 20) << " no cap on " << dn->inode->vino() << dendl;
     }
+
+    // In rare case during the rename if another thread tries to
+    // lookup the dst dentry, it may get an inconsistent result
+    // that both src dentry and dst dentry will link to the same
+    // inode at the same time.
+    // Will wait the rename to finish and try it again.
+    if (!is_rename && dn->is_renaming) {
+      ldout(cct, 1) << __func__ << " dir " << *dir
+                    << " rename is on the way, will wait for dn '"
+                    << dname << "'" << dendl;
+      wait_on_list(waiting_for_rename);
+      goto relookup;
+    }
   } else {
     // can we conclude ENOENT locally?
     if (dir->caps_issued_mask(CEPH_CAP_FILE_SHARED, true) &&
@@ -14202,12 +14221,13 @@ int Client::_rename(Inode *fromdir, const char *fromname, Inode *todir, const ch
     req->old_dentry_drop = CEPH_CAP_FILE_SHARED;
     req->old_dentry_unless = CEPH_CAP_FILE_EXCL;
 
+    de->is_renaming = true;
     req->set_dentry(de);
     req->dentry_drop = CEPH_CAP_FILE_SHARED;
     req->dentry_unless = CEPH_CAP_FILE_EXCL;
 
     InodeRef oldin, otherin;
-    res = _lookup(fromdir, fromname, 0, &oldin, perm);
+    res = _lookup(fromdir, fromname, 0, &oldin, perm, nullptr, true);
     if (res < 0)
       goto fail;
 
@@ -14216,7 +14236,7 @@ int Client::_rename(Inode *fromdir, const char *fromname, Inode *todir, const ch
     req->set_old_inode(oldinode);
     req->old_inode_drop = CEPH_CAP_LINK_SHARED;
 
-    res = _lookup(todir, toname, 0, &otherin, perm);
+    res = _lookup(todir, toname, 0, &otherin, perm, nullptr, true);
     switch (res) {
     case 0:
       {
@@ -14245,6 +14265,12 @@ int Client::_rename(Inode *fromdir, const char *fromname, Inode *todir, const ch
   res = make_request(req, perm, &target);
   ldout(cct, 10) << "rename result is " << res << dendl;
 
+  // if rename fails it will miss waking up the waiters
+  if (op == CEPH_MDS_OP_RENAME && de->is_renaming) {
+    de->is_renaming = false;
+    signal_cond_list(waiting_for_rename);
+  }
+
   // renamed item from our cache
 
   trim_cache();

diff --git a/src/client/Client.h b/src/client/Client.h
index 97c2ad733788dc3df1e31eb36c3d4bd142451a52..f71aab33f94051557c28dfa3d6a12f850891e557 100644 (file)
--- a/src/client/Client.h
+++ b/src/client/Client.h
@@ -1320,7 +1320,8 @@ private:
                 const UserPerm& perms);
 
   int _lookup(Inode *dir, const std::string& dname, int mask, InodeRef *target,
-             const UserPerm& perm, std::string* alternate_name=nullptr);
+             const UserPerm& perm, std::string* alternate_name=nullptr,
+              bool is_rename=false);
 
   int _link(Inode *in, Inode *dir, const char *name, const UserPerm& perm, std::string alternate_name,
            InodeRef *inp = 0);
@@ -1573,6 +1574,8 @@ private:
   std::map<std::pair<int64_t,std::string>, int> pool_perms;
   std::list<ceph::condition_variable*> waiting_for_pool_perm;
 
+  std::list<ceph::condition_variable*> waiting_for_rename;
+
   uint64_t retries_on_invalidate = 0;
 
   // state reclaim

diff --git a/src/client/Dentry.h b/src/client/Dentry.h
index 94722c5de70fde96ef25b92ab78e6ed891b7cb4a..8003dfed34be5d43427b8a04ecf3ab1d1c2089f5 100644 (file)
--- a/src/client/Dentry.h
+++ b/src/client/Dentry.h
@@ -91,6 +91,7 @@ public:
   ceph_seq_t lease_seq = 0;
   int cap_shared_gen = 0;
   std::string alternate_name;
+  bool is_renaming = false;
 
 private:
   xlist<Dentry *>::item inode_xlist_link;