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>

diff --git a/src/client/Client.cc b/src/client/Client.cc
index 7b77f1248ae7fd177328e0082e77705609388034..4c973ac19274b1586b6cfa2a9a6ef2fbdb6d3d27 100644 (file)
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -8579,22 +8579,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);
@@ -10671,31 +10655,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