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)

Conflicts:
	src/client/Client.cc : std::lock_guard() => Mutex::Locker()
			       ceph_assert() => assert()
			       dentries.empty() => dn_set.empty()

diff --git a/src/client/Client.cc b/src/client/Client.cc
index 0b240cbf787ec557a645415fe8f4b55dea1581dc..e9e862d37cdbe7ca930c98e1ecde1d72d98e3647 100644 (file)
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -8406,22 +8406,6 @@ int Client::_lookup_parent(Inode *ino, const UserPerm& perms, Inode **parent)
 {
   ldout(cct, 8) << "lookup_parent enter(" << ino->ino << ")" << dendl;
 
-  if (unmounting)
-    return -ENOTCONN;
-
-  if (!ino->dn_set.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) << "lookup_parent 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);
@@ -10475,31 +10459,38 @@ int Client::ll_lookup_inode(
     const UserPerm& perms,
     Inode **inode)
 {
+  assert(inode != NULL);
   Mutex::Locker 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;
-  }
-  assert(inode != NULL);
+
   assert(*inode != NULL);
 
+  if (!(*inode)->dn_set.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
+
   assert(parent != NULL);
 
   // Num3: Finally, get the name (dentry) of the requested inode