On Wed, 2016-08-10 at 18:21 -0400, Jeff Layton wrote:
> 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@redhat.
> > > > > com> wrote:
> > > > >
> > > > > On Wed, 2016-08-10 at 16:08 -0400, Patrick Donnelly wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > On Wed, Aug 10, 2016 at 12:30 PM, Jeff Layton <jlayto
> > > > > > > > > n@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 <j
> > > > > > > > > > > > > > > > > > > > > layton@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" ?
>
Thinking out loud...
I think we'd want that to return the info for "baz" since anything else
would mean escaping from cmount. So, looking at the code...maybe we
should alter Inode->is_root() to something like:
bool is_root() { return ino == client->get_root_ino; }
I don't think there are any callers of Inode->is_root currently, so I
wouldn't think this would break anything. Then we could call that to
see if we're at the root when doing a lookup or readdir of "..".
--
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
