On Wed, 2016-08-10 at 17:15 -0400, Patrick Donnelly wrote: > > On Wed, Aug 10, 2016 at 5:06 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > > > On Wed, 2016-08-10 at 16:46 -0400, Patrick Donnelly wrote: > > > > > > > > > > > On Wed, Aug 10, 2016 at 4:30 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > > > > > > > On Wed, 2016-08-10 at 16:08 -0400, Patrick Donnelly wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Aug 10, 2016 at 12:30 PM, Jeff Layton <jlayton@xxxxxxxxxx> > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > The CEPH_INO_DOTDOT thing is quite strange. Under most OS (Linux > > > > > > included), the parent of the root is itself. IOW, at the root, '.' > > > > > > and > > > > > > '..' refer to the same inode. > > > > > > > > > > > > Change the ceph client to do the same, as this allows users to get > > > > > > valid stat info for '..', as well as elimnating some special- > > > > > > casing. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > > > > > > > > > > Don't forget Client::_lookup: > > > > > > > > > > if (dname == "..") { > > > > > if (dir->dn_set.empty()) > > > > > r = -ENOENT; > > > > > else > > > > > *target = dir->get_first_parent()->dir->parent_inode; //dirs > > > > > can't be hard-linked > > > > > goto done; > > > > > } > > > > > > > > > > Otherwise LGTM. > > > > > > > > > > > > > > > > > Ahh, thanks. So will dir->dn_set.empty() be true at the root? If so, > > > > then something like the patch below? > > > > > > Well, that's tricky actually. My understanding is that if dn_set is > > > empty then either the inode is unlinked or it is the root inode (from > > > the client's perspective). So the below patch is probably not quite > > > right? I think if the directory is unlinked but not the root, its ".." > > > should still refer to its first parent? The ENOENT error is probably > > > wrong. > > > > > > > Ok, so is there some way to reliably tell whether it's the root? Should > > we instead check whether it's inode number is CEPH_INO_ROOT ? > > Inode::is_root should work. By the way, I see now that the readdir > code is also wrong. It should not need to check dn_set at all (just > is_root()). (It doesn't matter if the directory inode is unlinked, the > parent is still visible.) > Ahh thanks. I'll see about fixing that up while I'm in there too. Your point about is_root is valid, but I think we should step back a min and consider how we expect this to work when we mount a subtree of the MDS root. Suppose I do: ceph_mount(&cmount, "/foo/bar/baz"); ceph_lookup(&cmount, "/..", &st); ...what should we ultimately end up stat'ing there? Should I get back the info for "bar" or "baz" ? Thanks, -- Jeff Layton <jlayton@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html