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
	    - Mutex::Locker -> std::lock_guard
	    - changed some asserts to ceph_asserts

diff --git a/src/client/Client.cc b/src/client/Client.cc
index 020ad16c408486ab4e53907edf2df26144b8479a..13503ceffff3beac8e2bfefced409c4a2844386c 100644 (file)
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -8367,22 +8367,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);
@@ -10391,32 +10375,39 @@ int Client::ll_lookup_inode(
     const UserPerm& perms,
     Inode **inode)
 {
+  ceph_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;
+
+  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;
   }
-  assert(inode != NULL);
-  assert(*inode != NULL);
 
   // 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);
+
+  ceph_assert(parent != NULL);
 
   // Num3: Finally, get the name (dentry) of the requested inode
   r = _lookup_name(*inode, parent, perms);