[PATCH] register NFS_ACL with rpcbind

[PATCH] register NFS_ACL with rpcbind

[Peter Staubach]

Hi.

Here is a patch to modify the NFS server to register the NFS_ACL
services with the rpcbind daemon.  This allows the client to
ping for the existence of the NFS_ACL support via commands such
as "rpcinfo -t <server> nfs_acl".

The changelog for the patch which turned off this functionality
mentioned something about not registering the NFS_ACL as being
part of some tradition.  I can't find this tradition and the
only other implementation which supports NFS_ACL does register
them with the rpcbind daemon.

	Thanx...

		ps

Signed-off-by: Peter Staubach <staubach@redhat.com>

--- linux-2.6.31.i686/fs/nfsd/nfs2acl.c.org
+++ linux-2.6.31.i686/fs/nfsd/nfs2acl.c
@@ -346,5 +346,5 @@ struct svc_version	nfsd_acl_version2 = {
 		.vs_proc	= nfsd_acl_procedures2,
 		.vs_dispatch	= nfsd_dispatch,
 		.vs_xdrsize	= NFS3_SVC_XDRSIZE,
-		.vs_hidden	= 1,
+		.vs_hidden	= 0,
 };
--- linux-2.6.31.i686/fs/nfsd/nfs3acl.c.org
+++ linux-2.6.31.i686/fs/nfsd/nfs3acl.c
@@ -264,6 +264,6 @@ struct svc_version	nfsd_acl_version3 = {
 		.vs_proc	= nfsd_acl_procedures3,
 		.vs_dispatch	= nfsd_dispatch,
 		.vs_xdrsize	= NFS3_SVC_XDRSIZE,
-		.vs_hidden	= 1,
+		.vs_hidden	= 0,
 };
 

[PATCH v2] register NFS_ACL with rpcbind

[Peter Staubach]

Hi.

Here is a patch to modify the NFS server to register the NFS_ACL
services with the rpcbind daemon.  This allows the client to
ping for the existence of the NFS_ACL support via commands such
as "rpcinfo -t <server> nfs_acl".

This patch also modifies the NFS_ACL support so that responses
to version 2 NULLPROC requests can be made.

The changelog for the patch which turned off this functionality
mentioned something about not registering the NFS_ACL as being
part of some tradition.  I can't find this tradition and the
only other implementation which supports NFS_ACL does register
them with the rpcbind daemon.

	Thanx...

		ps

Signed-off-by: Peter Staubach <staubach@redhat.com>

--- linux-2.6.31.i686/fs/nfsd/nfs2acl.c.org
+++ linux-2.6.31.i686/fs/nfsd/nfs2acl.c
@@ -217,6 +217,16 @@ static int nfsaclsvc_decode_accessargs(s
  * XDR encode functions
  */
 
+/*
+ * There must be an encoding function for void results so svc_process
+ * will work properly.
+ */
+int
+nfsaclsvc_encode_voidres(struct svc_rqst *rqstp, __be32 *p, void *dummy)
+{
+	return xdr_ressize_check(rqstp, p);
+}
+
 /* GETACL */
 static int nfsaclsvc_encode_getaclres(struct svc_rqst *rqstp, __be32 *p,
 		struct nfsd3_getaclres *resp)
@@ -308,7 +318,6 @@ static int nfsaclsvc_release_access(stru
 }
 
 #define nfsaclsvc_decode_voidargs	NULL
-#define nfsaclsvc_encode_voidres	NULL
 #define nfsaclsvc_release_void		NULL
 #define nfsd3_fhandleargs	nfsd_fhandle
 #define nfsd3_attrstatres	nfsd_attrstat
@@ -346,5 +355,5 @@ struct svc_version	nfsd_acl_version2 = {
 		.vs_proc	= nfsd_acl_procedures2,
 		.vs_dispatch	= nfsd_dispatch,
 		.vs_xdrsize	= NFS3_SVC_XDRSIZE,
-		.vs_hidden	= 1,
+		.vs_hidden	= 0,
 };
--- linux-2.6.31.i686/fs/nfsd/nfs3acl.c.org
+++ linux-2.6.31.i686/fs/nfsd/nfs3acl.c
@@ -264,6 +264,6 @@ struct svc_version	nfsd_acl_version3 = {
 		.vs_proc	= nfsd_acl_procedures3,
 		.vs_dispatch	= nfsd_dispatch,
 		.vs_xdrsize	= NFS3_SVC_XDRSIZE,
-		.vs_hidden	= 1,
+		.vs_hidden	= 0,
 };
 

Re: [PATCH v2] register NFS_ACL with rpcbind

[Andreas Gruenbacher]

On Monday 02 November 2009 10:59:07 pm Peter Staubach wrote:
> Hi.
> 
> Here is a patch to modify the NFS server to register the NFS_ACL
> services with the rpcbind daemon.  This allows the client to
> ping for the existence of the NFS_ACL support via commands such
> as "rpcinfo -t <server> nfs_acl".
> 
> This patch also modifies the NFS_ACL support so that responses
> to version 2 NULLPROC requests can be made.
> 
> The changelog for the patch which turned off this functionality
> mentioned something about not registering the NFS_ACL as being
> part of some tradition.  I can't find this tradition and the
> only other implementation which supports NFS_ACL does register
> them with the rpcbind daemon.

I don't understand the reasoning behind .vs_hidden for NFS_ACL, hopefully Olaf 
can clarify. NFS_ACL is the only user of .vs_hidden as far as I can see 
though, so if this is changeg, shouldn't the entire commit bc5fea4 which 
introduced the flag be reverted?

Thanks,
Andreas

Re: [PATCH v2] register NFS_ACL with rpcbind

[Olaf Kirch]

On Tuesday 03 November 2009 10:13:27 Andreas Gruenbacher wrote:
> I don't understand the reasoning behind .vs_hidden for NFS_ACL, hopef=
ully
> Olaf can clarify. NFS_ACL is the only user of .vs_hidden as far as I =
can
> see though, so if this is changeg, shouldn't the entire commit bc5fea=
4
> which introduced the flag be reverted?

I can't remember the details of that one. I do remember that this is
based on someone's request who told me that we shouldn't register nfsac=
l
with portmap. I didn't check myself whether Solaris did or did not do
it at that time.

I have no issue with reverting that change, and removing the whole
=2Evs_hidden kludge too.

Thanks,
Olaf
--=20
Neo didn't bring down the Matrix. SOA did. (soafacts.com)
--------------------------------------------
Olaf Kirch - Director Server (okir@novell.com)
SUSE LINUX Products GmbH, Maxfeldstr. 5, D-90409 N=FCrnberg
GF: Markus Rex, HRB 16746 (AG N=FCrnberg)

Re: [PATCH v2] register NFS_ACL with rpcbind

[Peter Staubach]

Olaf Kirch wrote:
> On Tuesday 03 November 2009 10:13:27 Andreas Gruenbacher wrote:
>> I don't understand the reasoning behind .vs_hidden for NFS_ACL, hopefully
>> Olaf can clarify. NFS_ACL is the only user of .vs_hidden as far as I can
>> see though, so if this is changeg, shouldn't the entire commit bc5fea4
>> which introduced the flag be reverted?
> 
> I can't remember the details of that one. I do remember that this is
> based on someone's request who told me that we shouldn't register nfsacl
> with portmap. I didn't check myself whether Solaris did or did not do
> it at that time.
> 
> I have no issue with reverting that change, and removing the whole
> .vs_hidden kludge too.
> 

It seems that vs_hidden is used in 1 place outside of the NFS_ACL
server code.  It is used in the NFSv4 callback code.

I will look to see how difficult that might be to fix this spot
as well and then get rid of vs_hidden.

	Thanx...

		ps

Re: [PATCH v2] register NFS_ACL with rpcbind

[Chuck Lever]

On Nov 3, 2009, at 10:28 AM, Peter Staubach wrote:
> Olaf Kirch wrote:
>> On Tuesday 03 November 2009 10:13:27 Andreas Gruenbacher wrote:
>>> I don't understand the reasoning behind .vs_hidden for NFS_ACL,  
>>> hopefully
>>> Olaf can clarify. NFS_ACL is the only user of .vs_hidden as far as  
>>> I can
>>> see though, so if this is changeg, shouldn't the entire commit  
>>> bc5fea4
>>> which introduced the flag be reverted?
>>
>> I can't remember the details of that one. I do remember that this is
>> based on someone's request who told me that we shouldn't register  
>> nfsacl
>> with portmap. I didn't check myself whether Solaris did or did not do
>> it at that time.
>>
>> I have no issue with reverting that change, and removing the whole
>> .vs_hidden kludge too.
>>
>
> It seems that vs_hidden is used in 1 place outside of the NFS_ACL
> server code.  It is used in the NFSv4 callback code.
>
> I will look to see how difficult that might be to fix this spot
> as well and then get rid of vs_hidden.

See archive of this mailing list from earlier in October.  This change  
was added because it's hard to get rid of the svc_unregister() call  
done by svc_create().

I have another solution for that problem that I'm preparing for 2.6.33.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH v2] register NFS_ACL with rpcbind

[Peter Staubach]

Chuck Lever wrote:
> On Nov 3, 2009, at 10:28 AM, Peter Staubach wrote:
>> Olaf Kirch wrote:
>>> On Tuesday 03 November 2009 10:13:27 Andreas Gruenbacher wrote:
>>>> I don't understand the reasoning behind .vs_hidden for NFS_ACL,
>>>> hopefully
>>>> Olaf can clarify. NFS_ACL is the only user of .vs_hidden as far as I
>>>> can
>>>> see though, so if this is changeg, shouldn't the entire commit bc5fea4
>>>> which introduced the flag be reverted?
>>>
>>> I can't remember the details of that one. I do remember that this is
>>> based on someone's request who told me that we shouldn't register nfsacl
>>> with portmap. I didn't check myself whether Solaris did or did not do
>>> it at that time.
>>>
>>> I have no issue with reverting that change, and removing the whole
>>> .vs_hidden kludge too.
>>>
>>
>> It seems that vs_hidden is used in 1 place outside of the NFS_ACL
>> server code.  It is used in the NFSv4 callback code.
>>
>> I will look to see how difficult that might be to fix this spot
>> as well and then get rid of vs_hidden.
> 
> See archive of this mailing list from earlier in October.  This change
> was added because it's hard to get rid of the svc_unregister() call done
> by svc_create().
> 
> I have another solution for that problem that I'm preparing for 2.6.33.
> 

Cool.

In the meantime, can we get this one in, Bruce?

	Thanx...

		ps

Re: [PATCH v2] register NFS_ACL with rpcbind

[J. Bruce Fields]

On Mon, Nov 02, 2009 at 04:59:07PM -0500, Peter Staubach wrote:
> Hi.
> 
> Here is a patch to modify the NFS server to register the NFS_ACL
> services with the rpcbind daemon.  This allows the client to
> ping for the existence of the NFS_ACL support via commands such
> as "rpcinfo -t <server> nfs_acl".
> 
> This patch also modifies the NFS_ACL support so that responses
> to version 2 NULLPROC requests can be made.
> 
> The changelog for the patch which turned off this functionality
> mentioned something about not registering the NFS_ACL as being
> part of some tradition.  I can't find this tradition and the
> only other implementation which supports NFS_ACL does register
> them with the rpcbind daemon.
> 
> 	Thanx...
> 
> 		ps
> 
> Signed-off-by: Peter Staubach <staubach@redhat.com>
> 
> --- linux-2.6.31.i686/fs/nfsd/nfs2acl.c.org
> +++ linux-2.6.31.i686/fs/nfsd/nfs2acl.c
> @@ -217,6 +217,16 @@ static int nfsaclsvc_decode_accessargs(s
>   * XDR encode functions
>   */
>  
> +/*
> + * There must be an encoding function for void results so svc_process
> + * will work properly.
> + */
> +int
> +nfsaclsvc_encode_voidres(struct svc_rqst *rqstp, __be32 *p, void *dummy)
> +{
> +	return xdr_ressize_check(rqstp, p);
> +}

Out of curiosity: have you tested a null rpc?

Also, doesn't v3 have the same problem?

--b.

> +
>  /* GETACL */
>  static int nfsaclsvc_encode_getaclres(struct svc_rqst *rqstp, __be32 *p,
>  		struct nfsd3_getaclres *resp)
> @@ -308,7 +318,6 @@ static int nfsaclsvc_release_access(stru
>  }
>  
>  #define nfsaclsvc_decode_voidargs	NULL
> -#define nfsaclsvc_encode_voidres	NULL
>  #define nfsaclsvc_release_void		NULL
>  #define nfsd3_fhandleargs	nfsd_fhandle
>  #define nfsd3_attrstatres	nfsd_attrstat
> @@ -346,5 +355,5 @@ struct svc_version	nfsd_acl_version2 = {
>  		.vs_proc	= nfsd_acl_procedures2,
>  		.vs_dispatch	= nfsd_dispatch,
>  		.vs_xdrsize	= NFS3_SVC_XDRSIZE,
> -		.vs_hidden	= 1,
> +		.vs_hidden	= 0,
>  };
> --- linux-2.6.31.i686/fs/nfsd/nfs3acl.c.org
> +++ linux-2.6.31.i686/fs/nfsd/nfs3acl.c
> @@ -264,6 +264,6 @@ struct svc_version	nfsd_acl_version3 = {
>  		.vs_proc	= nfsd_acl_procedures3,
>  		.vs_dispatch	= nfsd_dispatch,
>  		.vs_xdrsize	= NFS3_SVC_XDRSIZE,
> -		.vs_hidden	= 1,
> +		.vs_hidden	= 0,
>  };
>  

Re: [PATCH v2] register NFS_ACL with rpcbind

[Chuck Lever]

On Nov 4, 2009, at 1:44 PM, Peter Staubach wrote:
> Chuck Lever wrote:
>> On Nov 3, 2009, at 10:28 AM, Peter Staubach wrote:
>>> Olaf Kirch wrote:
>>>> On Tuesday 03 November 2009 10:13:27 Andreas Gruenbacher wrote:
>>>>> I don't understand the reasoning behind .vs_hidden for NFS_ACL,
>>>>> hopefully
>>>>> Olaf can clarify. NFS_ACL is the only user of .vs_hidden as far  
>>>>> as I
>>>>> can
>>>>> see though, so if this is changeg, shouldn't the entire commit  
>>>>> bc5fea4
>>>>> which introduced the flag be reverted?
>>>>
>>>> I can't remember the details of that one. I do remember that this  
>>>> is
>>>> based on someone's request who told me that we shouldn't register  
>>>> nfsacl
>>>> with portmap. I didn't check myself whether Solaris did or did  
>>>> not do
>>>> it at that time.
>>>>
>>>> I have no issue with reverting that change, and removing the whole
>>>> .vs_hidden kludge too.
>>>>
>>>
>>> It seems that vs_hidden is used in 1 place outside of the NFS_ACL
>>> server code.  It is used in the NFSv4 callback code.
>>>
>>> I will look to see how difficult that might be to fix this spot
>>> as well and then get rid of vs_hidden.
>>
>> See archive of this mailing list from earlier in October.  This  
>> change
>> was added because it's hard to get rid of the svc_unregister() call  
>> done
>> by svc_create().
>>
>> I have another solution for that problem that I'm preparing for  
>> 2.6.33.
>>
>
> Cool.
>
> In the meantime, can we get this one in, Bruce?

As far as I know, there is no "meantime" in this case.  2.6.33 is the  
next merge window.

I don't have a problem with getting rid of .vs_hidden anyway... all  
I'm saying is maybe you don't have to work too hard at it.  :-)

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH v2] register NFS_ACL with rpcbind

[Peter Staubach]

J. Bruce Fields wrote:
> On Mon, Nov 02, 2009 at 04:59:07PM -0500, Peter Staubach wrote:
>> Hi.
>>
>> Here is a patch to modify the NFS server to register the NFS_ACL
>> services with the rpcbind daemon.  This allows the client to
>> ping for the existence of the NFS_ACL support via commands such
>> as "rpcinfo -t <server> nfs_acl".
>>
>> This patch also modifies the NFS_ACL support so that responses
>> to version 2 NULLPROC requests can be made.
>>
>> The changelog for the patch which turned off this functionality
>> mentioned something about not registering the NFS_ACL as being
>> part of some tradition.  I can't find this tradition and the
>> only other implementation which supports NFS_ACL does register
>> them with the rpcbind daemon.
>>
>> 	Thanx...
>>
>> 		ps
>>
>> Signed-off-by: Peter Staubach <staubach@redhat.com>
>>
>> --- linux-2.6.31.i686/fs/nfsd/nfs2acl.c.org
>> +++ linux-2.6.31.i686/fs/nfsd/nfs2acl.c
>> @@ -217,6 +217,16 @@ static int nfsaclsvc_decode_accessargs(s
>>   * XDR encode functions
>>   */
>>  
>> +/*
>> + * There must be an encoding function for void results so svc_process
>> + * will work properly.
>> + */
>> +int
>> +nfsaclsvc_encode_voidres(struct svc_rqst *rqstp, __be32 *p, void *dummy)
>> +{
>> +	return xdr_ressize_check(rqstp, p);
>> +}
> 
> Out of curiosity: have you tested a null rpc?
> 

Yup.  "rpcinfo -t <server> nfs_acl" works like a charm
with this patch applied for both versions 2 and 3.

> Also, doesn't v3 have the same problem?
> 

Nope.  It already had a similar XDR routine.  Which is
similar to the NFS XDR routine to do the same sort of
thing.  I suppose that someday, we could clean them up,
but these routines aren't doing any harm for the moment.

	Thanx...

		ps

> --b.
> 
>> +
>>  /* GETACL */
>>  static int nfsaclsvc_encode_getaclres(struct svc_rqst *rqstp, __be32 *p,
>>  		struct nfsd3_getaclres *resp)
>> @@ -308,7 +318,6 @@ static int nfsaclsvc_release_access(stru
>>  }
>>  
>>  #define nfsaclsvc_decode_voidargs	NULL
>> -#define nfsaclsvc_encode_voidres	NULL
>>  #define nfsaclsvc_release_void		NULL
>>  #define nfsd3_fhandleargs	nfsd_fhandle
>>  #define nfsd3_attrstatres	nfsd_attrstat
>> @@ -346,5 +355,5 @@ struct svc_version	nfsd_acl_version2 = {
>>  		.vs_proc	= nfsd_acl_procedures2,
>>  		.vs_dispatch	= nfsd_dispatch,
>>  		.vs_xdrsize	= NFS3_SVC_XDRSIZE,
>> -		.vs_hidden	= 1,
>> +		.vs_hidden	= 0,
>>  };
>> --- linux-2.6.31.i686/fs/nfsd/nfs3acl.c.org
>> +++ linux-2.6.31.i686/fs/nfsd/nfs3acl.c
>> @@ -264,6 +264,6 @@ struct svc_version	nfsd_acl_version3 = {
>>  		.vs_proc	= nfsd_acl_procedures3,
>>  		.vs_dispatch	= nfsd_dispatch,
>>  		.vs_xdrsize	= NFS3_SVC_XDRSIZE,
>> -		.vs_hidden	= 1,
>> +		.vs_hidden	= 0,
>>  };
>>  

Re: [PATCH v2] register NFS_ACL with rpcbind

[J. Bruce Fields]

On Wed, Nov 04, 2009 at 02:54:12PM -0500, Peter Staubach wrote:
> J. Bruce Fields wrote:
> > Out of curiosity: have you tested a null rpc?
> > 
> 
> Yup.  "rpcinfo -t <server> nfs_acl" works like a charm
> with this patch applied for both versions 2 and 3.
> 
> > Also, doesn't v3 have the same problem?
> > 
> 
> Nope.  It already had a similar XDR routine.  Which is
> similar to the NFS XDR routine to do the same sort of
> thing.  I suppose that someday, we could clean them up,

Yes.

> but these routines aren't doing any harm for the moment.

OK.  Applied.

--b.