Re: [V9fs-developer] [PATCH] 9p: attach-per-user

["Eric Van Hensbergen" <ericvh@gmail.com>]

On 9/3/07, Latchesar Ionkov <lucho@ionkov.net> wrote:
>
> This patch improves the 9P2000 support by allowing every user to attach
> separately. The patch defines three modes of access (new mount option
> 'access'):
>

nit picks:
   * you added/changed options without updated Documentation/filesystems/9p.txt
   * you changed v9fs->extended to be part of a flags structure, that
should have been
       a separate patch
   * rename of options should have been done in a separate patch

> - attach-per-user (access=user)
>  If a user tries to access a file served by v9fs for the first time, v9fs
>  sends an attach command to the server (Tattach) specifying the user. If
>  the attach succeeds, the user can access the v9fs tree.
>  As there is no uname->uid (string->integer) mapping yet, this mode works
>  only with the 9P2000.u dialect.
>
> - allow only one user to access the tree (access=<uid>)
>  Only the user with uid can access the v9fs tree. Other users that attempt
>  to access it will get EPERM error.
>
> - do all operations as a single user (access=any)
>  V9fs does a single attach and all operations are done as a single user.
>  If this mode is selected, the v9fs behavior is identical with the current
>  one.
>

Which option is default?

>
>  /**
> - * v9fs_fid_insert - add a fid to a dentry
> + * v9fs_fid_add - add a fid to a dentry
> + * @dentry: dentry that the fid is being added to
>   * @fid: fid to add
> - * @dentry: dentry that it is being added to
>   *
>   */
>
> @@ -66,52 +67,144 @@ int v9fs_fid_add(struct dentry *dentry, struct p9_fid *fid)
>  }

Even small cleanups like this should probably be confined to a
separate patch if they are unrelated.

>
> -struct p9_fid *v9fs_fid_lookup(struct dentry *dentry)
> +static struct p9_fid *v9fs_fid_find(struct dentry *dentry, u32 uid, int any)
>  {
...
>
> -struct p9_fid *v9fs_fid_clone(struct dentry *dentry)
> +struct p9_fid *v9fs_fid_lookup(struct dentry *dentry)
>  {
...
> +
> +struct p9_fid *v9fs_fid_clone(struct dentry *dentry)
> +{

The way the patch got formatted, these look like compulsive
renames..but there's an added function and then changes to the other
two.  I think it might be because of the way you ordered the
functions.  Put new functions after the old functions and maybe this
won't happen.  And clone seems to have lost his function header.  The
code is pretty inconsistent about those these days, but I'd like to do
an audit soon and make sure we have proper comment blocks where
appropriate.

scripts/checkpatch.pl reports:

ERROR: need a space before the open parenthesis '('
#244: FILE: fs/9p/fid.c:147:
+               for(ds = dentry; !IS_ROOT(ds); ds = ds->d_parent)

ERROR: need a space before the open parenthesis '('
#275: FILE: fs/9p/fid.c:178:
+       for(d = dentry, i = n; i >= 0; i--, d = d->d_parent)

Please fix up these small bits and resubmit.

                 -eric


Also, go ahead and cc: me directly on patches, for some reason this
one missed my normal filters and got lost.  If I'm directly cc:'d it
will pop to the top of my inbox.

           -eric