On Wed, 2016-08-10 at 21:23 +0000, Sage Weil wrote:
> On Wed, 10 Aug 2016, Jeff Layton 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?
> >
> > Note that this patch is not strictly necessary, but it does simplify
> > some other changes that I have queued up:
> >
> > diff --git a/src/client/Client.cc b/src/client/Client.cc
> > index 5ab0ace4d3df..287baaf20536 100644
> > --- a/src/client/Client.cc
> > +++ b/src/client/Client.cc
> > @@ -5924,7 +5924,7 @@ int Client::_lookup(Inode *dir, const string& dname, int mask,
> >
> > if (dname == "..") {
> > if (dir->dn_set.empty())
> > - r = -ENOENT;
> > + *target = dir;
> > else
> > *target = dir->get_first_parent()->dir->parent_inode; //dirs can't be hard-linked
> > goto done;
>
> IIRC I did the dotdot thing originally because otherwise the '..' entry at
> the mount point in ls -al didn't point to the parent directory. Having
> the fs explicitly do .. at all seems pretty weird to me... it seems like
> the VFS should be doing this. But in any case, I'd just verify that it
> behaves the same way a real mount does after this change.
>
> sage
The Linux VFS will definitely already handle ".." correctly (as you end
up doing a transition to a different vfsmount). So this shouldn't
affect ceph-fuse, AFAICT.
I think this change would primarily be noticed by those using libcephfs
directly...either in ceph_readdir/ceph_lookup (and related) calls, or
during a pathwalk.
That said, Patrick's suggestion to add some tests around this makes
sense. I'll plan to spin that up so we can be clear on how the behavior
changes.
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
