]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
mds: use projected path construction for access 12063/head
authorPatrick Donnelly <pdonnell@redhat.com>
Fri, 18 Nov 2016 02:05:03 +0000 (21:05 -0500)
committerPatrick Donnelly <pdonnell@redhat.com>
Sat, 19 Nov 2016 04:40:22 +0000 (23:40 -0500)
A new CDentry will not have a parent until its projected parent is
flushed to journal. During path construction a given dentry may have no
parent yet which will cause fallbacks to be used (the inode number).
This can cause access checks to fail:

    2016-11-17 19:50:43.830207 7eff9977a700 20 Session check_access path #10000000002/3

compare to an earlier check:

    2016-11-17 19:50:43.824223 7eff9977a700 20 Session check_access path /test/1/2

This commit refactors path construction to optionally use projected
parents for the entire chain of directories. Existing use of real stable
parents is unchanged. For example, this line is the same before and
after the patch:

    2016-11-18 23:17:15.611680 7f153f97a700 12 mds.0.cache.dir(10000000002) add_null_dentry [dentry #10000000002/3 [2,head] auth NULL (dversion lock) pv=0 v=1 inode=0 0x55e0f771f5f0]

Here inode "#10000000002" has no stable parent yet. So the path is
constructed as "#10000000002/3".

One notable change in this commit is the removal of
make_path_string_projected which was only used in debugging code. Here's
an example difference:

    2016-11-17 19:50:43.827915 7eff9977a700 10 mds.0.server traverse_to_auth_dir [dir 10000000003 {#10000000003 #10000000002/3}/ [2,head] auth v=1 cv=0/0 state=1073741826|complete f() n() hs=0+0,ss=0+0 0x55f5d35e2ee0]

to:

    2016-11-18 23:17:15.617757 7f153f97a700 10 mds.0.server traverse_to_auth_dir [dir 10000000003 /test/1/2/3/ [2,head] auth v=1 cv=0/0 state=1073741826|complete f() n() hs=0+0,ss=0+0 0x55e0f7706ee0]

Fixes: http://tracker.ceph.com/issues/17858
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
src/mds/CDentry.cc
src/mds/CDentry.h
src/mds/CDir.cc
src/mds/CInode.cc
src/mds/CInode.h
src/mds/ScrubStack.cc
src/mds/SessionMap.cc

index 2b953a03c3491d0f911dacae0a3880559fd97524..c842a6555b133448b918880d7fda94a291662440 100644 (file)
@@ -204,10 +204,10 @@ void CDentry::mark_new()
   state_set(STATE_NEW);
 }
 
-void CDentry::make_path_string(string& s) const
+void CDentry::make_path_string(string& s, bool projected) const
 {
   if (dir) {
-    dir->inode->make_path_string(s);
+    dir->inode->make_path_string(s, projected);
   } else {
     s = "???";
   }
@@ -215,35 +215,13 @@ void CDentry::make_path_string(string& s) const
   s.append(name.data(), name.length());
 }
 
-void CDentry::make_path(filepath& fp) const
+void CDentry::make_path(filepath& fp, bool projected) const
 {
   assert(dir);
-  if (dir->inode->is_base())
-    fp = filepath(dir->inode->ino());               // base case
-  else if (dir->inode->get_parent_dn())
-    dir->inode->get_parent_dn()->make_path(fp);  // recurse
-  else
-    fp = filepath(dir->inode->ino());               // relative but not base?  hrm!
+  dir->inode->make_path(fp, projected);
   fp.push_dentry(name);
 }
 
-/*
-void CDentry::make_path(string& s, inodeno_t tobase)
-{
-  assert(dir);
-  
-  if (dir->inode->is_root()) {
-    s += "/";  // make it an absolute path (no matter what) if we hit the root.
-  } 
-  else if (dir->inode->get_parent_dn() &&
-          dir->inode->ino() != tobase) {
-    dir->inode->get_parent_dn()->make_path(s, tobase);
-    s += "/";
-  }
-  s += name;
-}
-*/
-
 /*
  * we only add ourselves to remote_parents when the linkage is
  * active (no longer projected).  if the passed dnl is projected,
index a6b647e5ae43629fe6d2d0d1d64b03e3b660c956..2c75f5f9440d5a2242090fd2bc39f8d4b2970651 100644 (file)
@@ -276,8 +276,8 @@ public:
   const CDentry& operator= (const CDentry& right);
 
   // misc
-  void make_path_string(std::string& s) const;
-  void make_path(filepath& fp) const;
+  void make_path_string(std::string& s, bool projected=false) const;
+  void make_path(filepath& fp, bool projected=false) const;
 
   // -- version --
   version_t get_version() const { return version; }
index 9d20d16de6f30ef0179538f3444eefa3546ce299..141b3fde1a84b149d24780518f3cdd5f5416445b 100644 (file)
@@ -3115,7 +3115,7 @@ bool CDir::scrub_local()
 std::string CDir::get_path() const
 {
   std::string path;
-  get_inode()->make_path_string_projected(path);
+  get_inode()->make_path_string(path, true);
   return path;
 }
 
index 6bf463fc2c903097471b559480df4d0c4d69edef..dad9be7f71bf5dc83b1c59eba7160f7947a1b60b 100644 (file)
@@ -101,7 +101,7 @@ int num_cinode_locks = sizeof(cinode_lock_info) / sizeof(cinode_lock_info[0]);
 ostream& operator<<(ostream& out, const CInode& in)
 {
   string path;
-  in.make_path_string_projected(path);
+  in.make_path_string(path, true);
 
   out << "[inode " << in.inode.ino;
   out << " [" 
@@ -813,61 +813,47 @@ bool CInode::is_projected_ancestor_of(CInode *other)
 }
 
 /*
- * If use_parent is NULL (it should be one of inode's projected parents),
- * we use it to make path string. Otherwise, we use inode's parent dentry
- * to make path string
+ * Because a non-directory inode may have multiple links, the use_parent
+ * argument allows selecting which parent to use for path construction. This
+ * argument is only meaningful for the final component (i.e. the first of the
+ * nested calls) because directories cannot have multiple hard links. If
+ * use_parent is NULL and projected is true, the primary parent's projected
+ * inode is used all the way up the path chain. Otherwise the primary parent
+ * stable inode is used.
  */
-void CInode::make_path_string(string& s, CDentry *use_parent) const
+void CInode::make_path_string(string& s, bool projected, const CDentry *use_parent) const
 {
-  if (!use_parent)
-    use_parent = parent;
+  if (!use_parent) {
+    use_parent = projected ? get_projected_parent_dn() : parent;
+  }
 
   if (use_parent) {
-    use_parent->make_path_string(s);
-  } 
-  else if (is_root()) {
-    s = "";  // root
-  } 
-  else if (is_mdsdir()) {
+    use_parent->make_path_string(s, projected);
+  } else if (is_root()) {
+    s = "";
+  } else if (is_mdsdir()) {
     char t[40];
     uint64_t eino(ino());
     eino -= MDS_INO_MDSDIR_OFFSET;
     snprintf(t, sizeof(t), "~mds%" PRId64, eino);
     s = t;
-  }
-  else {
+  } else {
     char n[40];
     uint64_t eino(ino());
     snprintf(n, sizeof(n), "#%" PRIx64, eino);
     s += n;
   }
 }
-void CInode::make_path_string_projected(string& s) const
-{
-  make_path_string(s);
-  
-  if (!projected_parent.empty()) {
-    string q;
-    q.swap(s);
-    s = "{" + q;
-    for (list<CDentry*>::const_iterator p = projected_parent.begin();
-        p != projected_parent.end();
-        ++p) {
-      string q;
-      make_path_string(q, *p);
-      s += " ";
-      s += q;
-    }
-    s += "}";
-  }
-}
 
-void CInode::make_path(filepath& fp) const
+void CInode::make_path(filepath& fp, bool projected) const
 {
-  if (parent) 
-    parent->make_path(fp);
-  else
+  const CDentry *use_parent = projected ? get_projected_parent_dn() : parent;
+  if (use_parent) {
+    assert(!is_base());
+    use_parent->make_path(fp, projected);
+  } else {
     fp = filepath(ino());
+  }
 }
 
 void CInode::name_stray_dentry(string& dname)
index ceab2e8949bb279b56107c7f696c08e9d50eb936..3980686235779775a9f07af6849170fc012b9903 100644 (file)
@@ -739,9 +739,9 @@ public:
 
   // -- misc -- 
   bool is_projected_ancestor_of(CInode *other);
-  void make_path_string(std::string& s, CDentry *use_parent=NULL) const;
-  void make_path_string_projected(std::string& s) const;
-  void make_path(filepath& s) const;
+
+  void make_path_string(std::string& s, bool projected=false, const CDentry *use_parent=NULL) const;
+  void make_path(filepath& s, bool projected=false) const;
   void name_stray_dentry(std::string& dname);
   
   // -- dirtyness --
index 3448a496627fa0dd1b09ac839688768bdd1848fa..afc91c3961bf754c482e885ecb40ac2fa175d453 100644 (file)
@@ -387,7 +387,7 @@ void ScrubStack::_validate_inode_done(CInode *in, int r,
   // Inform the cluster log if we found an error
   if (!result.passed_validation) {
     std::string path;
-    in->make_path_string_projected(path);
+    in->make_path_string(path, true);
     clog->warn() << "Scrub error on inode " << *in
                  << " (" << path << ") see " << g_conf->name
                  << " log for details";
index ddbefad46c079acb97df614c281c19d06890ec19..5ea655ea0f391c3729fbc57870ab7bc65aff232e 100644 (file)
@@ -894,7 +894,7 @@ int Session::check_access(CInode *in, unsigned mask,
     path = in->get_projected_inode()->stray_prior_path;
     dout(20) << __func__ << " stray_prior_path " << path << dendl;
   } else {
-    in->make_path_string(path, in->get_projected_parent_dn());
+    in->make_path_string(path, true);
     dout(20) << __func__ << " path " << path << dendl;
   }
   if (path.length())