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 a51516aa13ef0f5fa0386129de0a79b517e75ec8..4653de37395cbd4180fd030968eb23cea7e3a24a 100644 (file)
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -1208,6 +1208,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);
@@ -7004,7 +7009,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;
@@ -7083,6 +7089,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) &&
@@ -14073,12 +14092,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;
 
@@ -14087,7 +14107,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:
       {
@@ -14116,6 +14136,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 97b8f74692d5b845748e9c57f5965768ae7d54db..36656636f252102c5438acff3eaafa6d374bc68c 100644 (file)
--- a/src/client/Client.h
+++ b/src/client/Client.h
@@ -1317,7 +1317,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);
@@ -1564,6 +1565,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;