From: Jeff Layton <jlayton@redhat.com>
Date: Thu, 30 May 2019 19:56:53 +0000 (-0400)
Subject: client: clean up error checking and return of _lookup_parent
X-Git-Tag: v14.2.2~35^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F28612%2Fhead;p=ceph.git

client: clean up error checking and return of _lookup_parent

ll_lookup_inode can end up getting back 0 from _lookup_parent,
without zeroing out the parent pointer, which ends up remaining
uninitialized.

Fix this by moving most of the sanity checks in _lookup_parent
into ll_lookup_inode, and only have it issue the call to the MDS.

This also allows us to do the checks in a more sane order.

Fixes: http://tracker.ceph.com/issues/40085
Signed-off-by: Jeff Layton <jlayton@redhat.com>
(cherry picked from commit 3ade7c046c6c5eaf20517d0713a805a681128831)
---

diff --git a/src/client/Client.cc b/src/client/Client.cc
index 98e79044015a..5f5e04446677 100644
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -8577,22 +8577,6 @@ int Client::_lookup_parent(Inode *ino, const UserPerm& perms, Inode **parent)
 {
   ldout(cct, 8) << __func__ << " enter(" << ino->ino << ")" << dendl;
 
-  if (unmounting)
-    return -ENOTCONN;
-
-  if (!ino->dentries.empty()) {
-    // if we exposed the parent here, we'd need to check permissions,
-    // but right now we just rely on the MDS doing so in make_request
-    ldout(cct, 8) << __func__ << " dentry already present" << dendl;
-    return 0;
-  }
-  
-  if (ino->is_root()) {
-    *parent = NULL;
-    ldout(cct, 8) << "ino is root, no parent" << dendl;
-    return -EINVAL;
-  }
-
   MetaRequest *req = new MetaRequest(CEPH_MDS_OP_LOOKUPPARENT);
   filepath path(ino->ino);
   req->set_filepath(path);
@@ -10669,31 +10653,38 @@ int Client::ll_lookup_inode(
     const UserPerm& perms,
     Inode **inode)
 {
+  ceph_assert(inode != NULL);
   std::lock_guard lock(client_lock);
   ldout(cct, 3) << "ll_lookup_inode " << ino  << dendl;
    
+  if (unmounting)
+    return -ENOTCONN;
+
   // Num1: get inode and *inode
   int r = _lookup_ino(ino, perms, inode);
-  if (r) {
+  if (r)
     return r;
-  }
-  ceph_assert(inode != NULL);
+
   ceph_assert(*inode != NULL);
 
+  if (!(*inode)->dentries.empty()) {
+    ldout(cct, 8) << __func__ << " dentry already present" << dendl;
+    return 0;
+  }
+
+  if ((*inode)->is_root()) {
+    ldout(cct, 8) << "ino is root, no parent" << dendl;
+    return 0;
+  }
+
   // Num2: Request the parent inode, so that we can look up the name
   Inode *parent;
   r = _lookup_parent(*inode, perms, &parent);
-  if (r && r != -EINVAL) {
-    // Unexpected error
+  if (r) {
     _ll_forget(*inode, 1);  
     return r;
-  } else if (r == -EINVAL) {
-    // EINVAL indicates node without parents (root), drop out now
-    // and don't try to look up the non-existent dentry.
-    return 0;
   }
-  // FIXME: I don't think this works; lookup_parent() returns 0 if the parent
-  // is already in cache
+
   ceph_assert(parent != NULL);
 
   // Num3: Finally, get the name (dentry) of the requested inode