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)

Conflicts:
	src/client/Client.h: a minor conflict.

diff --git a/src/client/Client.cc b/src/client/Client.cc
index 52980d5ecb07ca185b4a154bbb11b69a7afcb643..2a270323c0f04f5bf926f76060e29d918d5007a6 100644 (file)
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -1199,6 +1199,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);
@@ -7021,7 +7026,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;
@@ -7100,6 +7106,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) &&
@@ -14056,12 +14075,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;
 
@@ -14070,7 +14090,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:
       {
@@ -14099,6 +14119,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 939602cd05e6eef06458a6392f17d2073d3ea157..a2a8ed8d0782994e6804c96c43d06c1d9e3b55fc 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 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);
@@ -1563,6 +1564,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 483a31eccb324b56dd704f1931dbe1a68cea26e6..63669609caacc75687d2321993b420ac6ff2e814 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;