Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 16 Feb 2018 09:38:45 -0700
From:      Alan Somers <asomers@freebsd.org>
To:        John Baldwin <jhb@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org,  svn-src-head@freebsd.org
Subject:   Re: svn commit: r329265 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
Message-ID:  <CAOtMX2idkGYqtEQiP9DGGbk9ptcbR95mkkhNy4D3jQ11Jz4mhw@mail.gmail.com>
In-Reply-To: <3148002.4yJvN52HMT@ralph.baldwin.cx>
References:  <201802141549.w1EFnVBV064848@repo.freebsd.org> <3148002.4yJvN52HMT@ralph.baldwin.cx>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Feb 15, 2018 at 12:18 PM, John Baldwin <jhb@freebsd.org> wrote:

> On Wednesday, February 14, 2018 03:49:31 PM Alan Somers wrote:
> > Author: asomers
> > Date: Wed Feb 14 15:49:31 2018
> > New Revision: 329265
> > URL: https://svnweb.freebsd.org/changeset/base/329265
> >
> > Log:
> >   Implement .vop_pathconf and .vop_getacl for the .zfs ctldir
> >
> >   zfsctl_common_pathconf will report all the same variables that regular
> ZFS
> >   volumes report. zfsctl_common_getacl will report an ACL equivalent to
> 555,
> >   except that you can't read xattrs or edit attributes.
> >
> >   Fixes a bug where "ls .zfs" will occasionally print something like:
> >   ls: .zfs/.: Operation not supported
> >
> >   PR:         225793
> >   Reviewed by:        avg
> >   MFC after:  3 weeks
> >   Sponsored by:       Spectra Logic Corp
> >   Differential Revision:      https://reviews.freebsd.org/D14365
> >
> > Modified:
> >   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c
> >
> > Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/
> zfs_ctldir.c
> > ============================================================
> ==================
> > --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c
> Wed Feb 14 15:40:13 2018        (r329264)
> > +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c
> Wed Feb 14 15:49:31 2018        (r329265)
> > @@ -80,6 +80,10 @@
> >
> >  #include "zfs_namecheck.h"
> >
> > +/* Common access mode for all virtual directories under the ctldir */
> > +const u_short zfsctl_ctldir_mode = S_IRUSR | S_IXUSR | S_IRGRP |
> S_IXGRP |
> > +    S_IROTH | S_IXOTH;
> > +
> >  /*
> >   * "Synthetic" filesystem implementation.
> >   */
> > @@ -496,8 +500,7 @@ zfsctl_common_getattr(vnode_t *vp, vattr_t *vap)
> >       vap->va_nblocks = 0;
> >       vap->va_seq = 0;
> >       vn_fsid(vp, vap);
> > -     vap->va_mode = S_IRUSR | S_IXUSR | S_IRGRP | S_IXGRP |
> > -         S_IROTH | S_IXOTH;
> > +     vap->va_mode = zfsctl_ctldir_mode;
> >       vap->va_type = VDIR;
> >       /*
> >        * We live in the now (for atime).
> > @@ -724,6 +727,87 @@ zfsctl_root_vptocnp(struct vop_vptocnp_args *ap)
> >       return (0);
> >  }
> >
> > +static int
> > +zfsctl_common_pathconf(ap)
> > +     struct vop_pathconf_args /* {
> > +             struct vnode *a_vp;
> > +             int a_name;
> > +             int *a_retval;
> > +     } */ *ap;
> > +{
> > +     /*
> > +      * We care about ACL variables so that user land utilities like ls
> > +      * can display them correctly.  Since the ctldir's st_dev is set
> to be
> > +      * the same as the parent dataset, we must support all variables
> that
> > +      * it supports.
> > +      */
> > +     switch (ap->a_name) {
> > +     case _PC_LINK_MAX:
> > +             *ap->a_retval = INT_MAX;
> > +             return (0);
>
> On HEAD this should probably match the existing ZFS pathconf
> (min(LONG_MAX, ZFS_LINK_MAX IIRC), though if these directories can only
> ever have a link count of 2 (which seems true from zfsctl_common_getattr())
> then it would be fine to return '2' here instead.  For stable you'd have to
> restrict it to LINK_MAX if you don't use '2'.
>
> Also, you should call vfs_stdpathconf() in the default: case.
>
> --
> John Baldwin
>

All good points.  I don't think a max link count of 2 is correct, though,
because the real link count can be higher than that.  See
zfsctl_root_getattr and zfsctl_snapdir_getattr.  I'll set it the way that
zfs_pathconf does.  Also, it looks like I need to add _PC_NAME_MAX.
-Alan



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAOtMX2idkGYqtEQiP9DGGbk9ptcbR95mkkhNy4D3jQ11Jz4mhw>