mirror of
https://github.com/ceph/ceph
synced 2025-01-11 21:50:26 +00:00
mds: use projected path construction for access
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>
This commit is contained in:
parent
64cc94c522
commit
7b4561091e
@ -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,
|
||||
|
@ -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; }
|
||||
|
@ -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;
|
||||
}
|
||||
|
||||
|
@ -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)
|
||||
|
@ -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 --
|
||||
|
@ -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";
|
||||
|
@ -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())
|
||||
|
Loading…
Reference in New Issue
Block a user