2008-02-12 21:23:43

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 07/11] NFSD: Use "depends on" for PROC_FS dependency

On Mon, Feb 11, 2008 at 05:12:16PM -0500, Chuck Lever wrote:
> Recently, commit 440bcc59 added a reverse dependency to fs/Kconfig to
> ensure that PROC_FS was enabled if NFSD_V4 was enabled.
>
> There is a guideline in Documentation/kbuild/kconfig-language.txt that
> states "In general use select only for non-visible symbols (no prompts
> anywhere) and for symbols with no dependencies."
>
> A quick grep around other Kconfig files reveals that no entry currently
> uses "select PROC_FS" -- every one uses "depends on". Thus CONFIG_NFSD_V4
> should use "depends on PROC_FS" as well.

OK.

>
> For SUNRPC_GSS, it's a little more complex. Other entries can "select"
> SUNRPC_GSS, as it is non-visible. However, the guideline suggests an
> entry can't "select" it if it has a dependency (such as PROC_FS).
> So, we add forward dependencies on PROC_FS to RPCSEC_GSS_FOO instead.

This also makes the client dependent on PROC_FS, which it wasn't before.

I assume nobody cares?

--b.

>
> XXX: Both CONFIG_NFSV4 and CONFIG_NFSD_V4 select RPCSEC_GSS_KRB5, which is
> visible, which kconfig-language.txt also frowns upon. The intent was to
> enable at least one GSS mechanism if V4 was enabled. Perhaps we should
> make SUNRPC_GSS visible, and make the NFSv4 options visible only if
> SUNRPC_GSS is enabled.
>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
>
> fs/Kconfig | 8 +++-----
> 1 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/fs/Kconfig b/fs/Kconfig
> index 0c8f85d..9ee288f 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -1662,8 +1662,6 @@ config NFSD
> select CRYPTO_MD5 if NFSD_V4
> select CRYPTO if NFSD_V4
> select FS_POSIX_ACL if NFSD_V4
> - select PROC_FS if NFSD_V4
> - select PROC_FS if SUNRPC_GSS
> help
> Choose Y here if you want to allow other computers to access
> files residing on this system using Sun's Network File System
> @@ -1723,7 +1721,7 @@ config NFSD_V3_ACL
>
> config NFSD_V4
> bool "NFS server support for NFS version 4 (EXPERIMENTAL)"
> - depends on NFSD && NFSD_V3 && EXPERIMENTAL
> + depends on NFSD && NFSD_V3 && PROC_FS && EXPERIMENTAL
> select RPCSEC_GSS_KRB5
> help
> This option enables support in your system's NFS server for
> @@ -1798,7 +1796,7 @@ config SUNRPC_BIND34
>
> config RPCSEC_GSS_KRB5
> tristate "Secure RPC: Kerberos V mechanism (EXPERIMENTAL)"
> - depends on SUNRPC && EXPERIMENTAL
> + depends on SUNRPC && PROC_FS && EXPERIMENTAL
> select SUNRPC_GSS
> select CRYPTO
> select CRYPTO_MD5
> @@ -1817,7 +1815,7 @@ config RPCSEC_GSS_KRB5
>
> config RPCSEC_GSS_SPKM3
> tristate "Secure RPC: SPKM3 mechanism (EXPERIMENTAL)"
> - depends on SUNRPC && EXPERIMENTAL
> + depends on SUNRPC && PROC_FS && EXPERIMENTAL
> select SUNRPC_GSS
> select CRYPTO
> select CRYPTO_MD5
>


2008-02-12 21:47:28

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH 07/11] NFSD: Use "depends on" for PROC_FS dependency


On Tue, 2008-02-12 at 16:23 -0500, J. Bruce Fields wrote:
> On Mon, Feb 11, 2008 at 05:12:16PM -0500, Chuck Lever wrote:
> > Recently, commit 440bcc59 added a reverse dependency to fs/Kconfig to
> > ensure that PROC_FS was enabled if NFSD_V4 was enabled.
> >
> > There is a guideline in Documentation/kbuild/kconfig-language.txt that
> > states "In general use select only for non-visible symbols (no prompts
> > anywhere) and for symbols with no dependencies."
> >
> > A quick grep around other Kconfig files reveals that no entry currently
> > uses "select PROC_FS" -- every one uses "depends on". Thus CONFIG_NFSD_V4
> > should use "depends on PROC_FS" as well.
>
> OK.
>
> >
> > For SUNRPC_GSS, it's a little more complex. Other entries can "select"
> > SUNRPC_GSS, as it is non-visible. However, the guideline suggests an
> > entry can't "select" it if it has a dependency (such as PROC_FS).
> > So, we add forward dependencies on PROC_FS to RPCSEC_GSS_FOO instead.
>
> This also makes the client dependent on PROC_FS, which it wasn't before.
>
> I assume nobody cares?

You assume wrongly. Patch NAKed...

Trond


2008-02-12 22:11:28

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 07/11] NFSD: Use "depends on" for PROC_FS dependency


On Feb 12, 2008, at 4:23 PM, J. Bruce Fields wrote:

> On Mon, Feb 11, 2008 at 05:12:16PM -0500, Chuck Lever wrote:
>> Recently, commit 440bcc59 added a reverse dependency to fs/Kconfig to
>> ensure that PROC_FS was enabled if NFSD_V4 was enabled.
>>
>> There is a guideline in Documentation/kbuild/kconfig-language.txt
>> that
>> states "In general use select only for non-visible symbols (no
>> prompts
>> anywhere) and for symbols with no dependencies."
>>
>> A quick grep around other Kconfig files reveals that no entry
>> currently
>> uses "select PROC_FS" -- every one uses "depends on". Thus
>> CONFIG_NFSD_V4
>> should use "depends on PROC_FS" as well.
>
> OK.
>
>>
>> For SUNRPC_GSS, it's a little more complex. Other entries can
>> "select"
>> SUNRPC_GSS, as it is non-visible. However, the guideline suggests an
>> entry can't "select" it if it has a dependency (such as PROC_FS).
>> So, we add forward dependencies on PROC_FS to RPCSEC_GSS_FOO instead.
>
> This also makes the client dependent on PROC_FS, which it wasn't
> before.
>
> I assume nobody cares?

Huh? If I disable PROC_FS, I can still select the NFS client
options, even NFSv4.

>> XXX: Both CONFIG_NFSV4 and CONFIG_NFSD_V4 select RPCSEC_GSS_KRB5,
>> which is
>> visible, which kconfig-language.txt also frowns upon. The intent
>> was to
>> enable at least one GSS mechanism if V4 was enabled. Perhaps we
>> should
>> make SUNRPC_GSS visible, and make the NFSv4 options visible only if
>> SUNRPC_GSS is enabled.
>>
>> Signed-off-by: Chuck Lever <[email protected]>
>> ---
>>
>> fs/Kconfig | 8 +++-----
>> 1 files changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/Kconfig b/fs/Kconfig
>> index 0c8f85d..9ee288f 100644
>> --- a/fs/Kconfig
>> +++ b/fs/Kconfig
>> @@ -1662,8 +1662,6 @@ config NFSD
>> select CRYPTO_MD5 if NFSD_V4
>> select CRYPTO if NFSD_V4
>> select FS_POSIX_ACL if NFSD_V4
>> - select PROC_FS if NFSD_V4
>> - select PROC_FS if SUNRPC_GSS
>> help
>> Choose Y here if you want to allow other computers to access
>> files residing on this system using Sun's Network File System
>> @@ -1723,7 +1721,7 @@ config NFSD_V3_ACL
>>
>> config NFSD_V4
>> bool "NFS server support for NFS version 4 (EXPERIMENTAL)"
>> - depends on NFSD && NFSD_V3 && EXPERIMENTAL
>> + depends on NFSD && NFSD_V3 && PROC_FS && EXPERIMENTAL
>> select RPCSEC_GSS_KRB5
>> help
>> This option enables support in your system's NFS server for
>> @@ -1798,7 +1796,7 @@ config SUNRPC_BIND34
>>
>> config RPCSEC_GSS_KRB5
>> tristate "Secure RPC: Kerberos V mechanism (EXPERIMENTAL)"
>> - depends on SUNRPC && EXPERIMENTAL
>> + depends on SUNRPC && PROC_FS && EXPERIMENTAL
>> select SUNRPC_GSS
>> select CRYPTO
>> select CRYPTO_MD5
>> @@ -1817,7 +1815,7 @@ config RPCSEC_GSS_KRB5
>>
>> config RPCSEC_GSS_SPKM3
>> tristate "Secure RPC: SPKM3 mechanism (EXPERIMENTAL)"
>> - depends on SUNRPC && EXPERIMENTAL
>> + depends on SUNRPC && PROC_FS && EXPERIMENTAL
>> select SUNRPC_GSS
>> select CRYPTO
>> select CRYPTO_MD5
>>

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




2008-02-12 22:30:42

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 07/11] NFSD: Use "depends on" for PROC_FS dependency

On Tue, Feb 12, 2008 at 05:10:57PM -0500, Chuck Lever wrote:
>
> On Feb 12, 2008, at 4:23 PM, J. Bruce Fields wrote:
>
>> On Mon, Feb 11, 2008 at 05:12:16PM -0500, Chuck Lever wrote:
>>> Recently, commit 440bcc59 added a reverse dependency to fs/Kconfig to
>>> ensure that PROC_FS was enabled if NFSD_V4 was enabled.
>>>
>>> There is a guideline in Documentation/kbuild/kconfig-language.txt
>>> that
>>> states "In general use select only for non-visible symbols (no
>>> prompts
>>> anywhere) and for symbols with no dependencies."
>>>
>>> A quick grep around other Kconfig files reveals that no entry
>>> currently
>>> uses "select PROC_FS" -- every one uses "depends on". Thus
>>> CONFIG_NFSD_V4
>>> should use "depends on PROC_FS" as well.
>>
>> OK.
>>
>>>
>>> For SUNRPC_GSS, it's a little more complex. Other entries can
>>> "select"
>>> SUNRPC_GSS, as it is non-visible. However, the guideline suggests an
>>> entry can't "select" it if it has a dependency (such as PROC_FS).
>>> So, we add forward dependencies on PROC_FS to RPCSEC_GSS_FOO instead.
>>
>> This also makes the client dependent on PROC_FS, which it wasn't
>> before.
>>
>> I assume nobody cares?
>
> Huh? If I disable PROC_FS, I can still select the NFS client options,
> even NFSv4.

OK. I'm confused, then:

NFS_V4 selects RPCSEC_GSS_KRB5
RPCSEC_GSS_KRB5 (after this patch) depends on PROC_FS

What am I missing?

--b.

>
>>> XXX: Both CONFIG_NFSV4 and CONFIG_NFSD_V4 select RPCSEC_GSS_KRB5,
>>> which is
>>> visible, which kconfig-language.txt also frowns upon. The intent
>>> was to
>>> enable at least one GSS mechanism if V4 was enabled. Perhaps we
>>> should
>>> make SUNRPC_GSS visible, and make the NFSv4 options visible only if
>>> SUNRPC_GSS is enabled.
>>>
>>> Signed-off-by: Chuck Lever <[email protected]>
>>> ---
>>>
>>> fs/Kconfig | 8 +++-----
>>> 1 files changed, 3 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/fs/Kconfig b/fs/Kconfig
>>> index 0c8f85d..9ee288f 100644
>>> --- a/fs/Kconfig
>>> +++ b/fs/Kconfig
>>> @@ -1662,8 +1662,6 @@ config NFSD
>>> select CRYPTO_MD5 if NFSD_V4
>>> select CRYPTO if NFSD_V4
>>> select FS_POSIX_ACL if NFSD_V4
>>> - select PROC_FS if NFSD_V4
>>> - select PROC_FS if SUNRPC_GSS
>>> help
>>> Choose Y here if you want to allow other computers to access
>>> files residing on this system using Sun's Network File System
>>> @@ -1723,7 +1721,7 @@ config NFSD_V3_ACL
>>>
>>> config NFSD_V4
>>> bool "NFS server support for NFS version 4 (EXPERIMENTAL)"
>>> - depends on NFSD && NFSD_V3 && EXPERIMENTAL
>>> + depends on NFSD && NFSD_V3 && PROC_FS && EXPERIMENTAL
>>> select RPCSEC_GSS_KRB5
>>> help
>>> This option enables support in your system's NFS server for
>>> @@ -1798,7 +1796,7 @@ config SUNRPC_BIND34
>>>
>>> config RPCSEC_GSS_KRB5
>>> tristate "Secure RPC: Kerberos V mechanism (EXPERIMENTAL)"
>>> - depends on SUNRPC && EXPERIMENTAL
>>> + depends on SUNRPC && PROC_FS && EXPERIMENTAL
>>> select SUNRPC_GSS
>>> select CRYPTO
>>> select CRYPTO_MD5
>>> @@ -1817,7 +1815,7 @@ config RPCSEC_GSS_KRB5
>>>
>>> config RPCSEC_GSS_SPKM3
>>> tristate "Secure RPC: SPKM3 mechanism (EXPERIMENTAL)"
>>> - depends on SUNRPC && EXPERIMENTAL
>>> + depends on SUNRPC && PROC_FS && EXPERIMENTAL
>>> select SUNRPC_GSS
>>> select CRYPTO
>>> select CRYPTO_MD5
>>>
>
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
>
>
>

2008-02-13 18:29:06

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 07/11] NFSD: Use "depends on" for PROC_FS dependency

J. Bruce Fields wrote:
> On Tue, Feb 12, 2008 at 05:10:57PM -0500, Chuck Lever wrote:
>> On Feb 12, 2008, at 4:23 PM, J. Bruce Fields wrote:
>>
>>> On Mon, Feb 11, 2008 at 05:12:16PM -0500, Chuck Lever wrote:
>>>> Recently, commit 440bcc59 added a reverse dependency to fs/Kconfig to
>>>> ensure that PROC_FS was enabled if NFSD_V4 was enabled.
>>>>
>>>> There is a guideline in Documentation/kbuild/kconfig-language.txt
>>>> that
>>>> states "In general use select only for non-visible symbols (no
>>>> prompts
>>>> anywhere) and for symbols with no dependencies."
>>>>
>>>> A quick grep around other Kconfig files reveals that no entry
>>>> currently
>>>> uses "select PROC_FS" -- every one uses "depends on". Thus
>>>> CONFIG_NFSD_V4
>>>> should use "depends on PROC_FS" as well.
>>> OK.
>>>
>>>> For SUNRPC_GSS, it's a little more complex. Other entries can
>>>> "select"
>>>> SUNRPC_GSS, as it is non-visible. However, the guideline suggests an
>>>> entry can't "select" it if it has a dependency (such as PROC_FS).
>>>> So, we add forward dependencies on PROC_FS to RPCSEC_GSS_FOO instead.
>>> This also makes the client dependent on PROC_FS, which it wasn't
>>> before.
>>>
>>> I assume nobody cares?
>> Huh? If I disable PROC_FS, I can still select the NFS client options,
>> even NFSv4.
>
> OK. I'm confused, then:
>
> NFS_V4 selects RPCSEC_GSS_KRB5
> RPCSEC_GSS_KRB5 (after this patch) depends on PROC_FS
>
> What am I missing?

It appears that the "select RPCSEC_GSS_KRB5" is a no-op if
RPCSEC_GSS_KRB5 hasn't met it's forward dependencies.

>>>> XXX: Both CONFIG_NFSV4 and CONFIG_NFSD_V4 select RPCSEC_GSS_KRB5,
>>>> which is
>>>> visible, which kconfig-language.txt also frowns upon. The intent
>>>> was to
>>>> enable at least one GSS mechanism if V4 was enabled. Perhaps we
>>>> should
>>>> make SUNRPC_GSS visible, and make the NFSv4 options visible only if
>>>> SUNRPC_GSS is enabled.
>>>>
>>>> Signed-off-by: Chuck Lever <[email protected]>
>>>> ---
>>>>
>>>> fs/Kconfig | 8 +++-----
>>>> 1 files changed, 3 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/fs/Kconfig b/fs/Kconfig
>>>> index 0c8f85d..9ee288f 100644
>>>> --- a/fs/Kconfig
>>>> +++ b/fs/Kconfig
>>>> @@ -1662,8 +1662,6 @@ config NFSD
>>>> select CRYPTO_MD5 if NFSD_V4
>>>> select CRYPTO if NFSD_V4
>>>> select FS_POSIX_ACL if NFSD_V4
>>>> - select PROC_FS if NFSD_V4
>>>> - select PROC_FS if SUNRPC_GSS
>>>> help
>>>> Choose Y here if you want to allow other computers to access
>>>> files residing on this system using Sun's Network File System
>>>> @@ -1723,7 +1721,7 @@ config NFSD_V3_ACL
>>>>
>>>> config NFSD_V4
>>>> bool "NFS server support for NFS version 4 (EXPERIMENTAL)"
>>>> - depends on NFSD && NFSD_V3 && EXPERIMENTAL
>>>> + depends on NFSD && NFSD_V3 && PROC_FS && EXPERIMENTAL
>>>> select RPCSEC_GSS_KRB5
>>>> help
>>>> This option enables support in your system's NFS server for
>>>> @@ -1798,7 +1796,7 @@ config SUNRPC_BIND34
>>>>
>>>> config RPCSEC_GSS_KRB5
>>>> tristate "Secure RPC: Kerberos V mechanism (EXPERIMENTAL)"
>>>> - depends on SUNRPC && EXPERIMENTAL
>>>> + depends on SUNRPC && PROC_FS && EXPERIMENTAL
>>>> select SUNRPC_GSS
>>>> select CRYPTO
>>>> select CRYPTO_MD5
>>>> @@ -1817,7 +1815,7 @@ config RPCSEC_GSS_KRB5
>>>>
>>>> config RPCSEC_GSS_SPKM3
>>>> tristate "Secure RPC: SPKM3 mechanism (EXPERIMENTAL)"
>>>> - depends on SUNRPC && EXPERIMENTAL
>>>> + depends on SUNRPC && PROC_FS && EXPERIMENTAL
>>>> select SUNRPC_GSS
>>>> select CRYPTO
>>>> select CRYPTO_MD5
>>>>
>> --
>> Chuck Lever
>> chuck[dot]lever[at]oracle[dot]com
>>
>>
>>


Attachments:
chuck_lever.vcf (315.00 B)

2008-02-13 18:59:27

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 07/11] NFSD: Use "depends on" for PROC_FS dependency

On Wed, Feb 13, 2008 at 01:28:29PM -0500, Chuck Lever wrote:
> J. Bruce Fields wrote:
>> On Tue, Feb 12, 2008 at 05:10:57PM -0500, Chuck Lever wrote:
>>> On Feb 12, 2008, at 4:23 PM, J. Bruce Fields wrote:
>>>
>>>> On Mon, Feb 11, 2008 at 05:12:16PM -0500, Chuck Lever wrote:
>>>>> Recently, commit 440bcc59 added a reverse dependency to fs/Kconfig to
>>>>> ensure that PROC_FS was enabled if NFSD_V4 was enabled.
>>>>>
>>>>> There is a guideline in Documentation/kbuild/kconfig-language.txt
>>>>> that
>>>>> states "In general use select only for non-visible symbols (no
>>>>> prompts
>>>>> anywhere) and for symbols with no dependencies."
>>>>>
>>>>> A quick grep around other Kconfig files reveals that no entry
>>>>> currently
>>>>> uses "select PROC_FS" -- every one uses "depends on". Thus
>>>>> CONFIG_NFSD_V4
>>>>> should use "depends on PROC_FS" as well.
>>>> OK.
>>>>
>>>>> For SUNRPC_GSS, it's a little more complex. Other entries can
>>>>> "select"
>>>>> SUNRPC_GSS, as it is non-visible. However, the guideline suggests an
>>>>> entry can't "select" it if it has a dependency (such as PROC_FS).
>>>>> So, we add forward dependencies on PROC_FS to RPCSEC_GSS_FOO instead.
>>>> This also makes the client dependent on PROC_FS, which it wasn't
>>>> before.
>>>>
>>>> I assume nobody cares?
>>> Huh? If I disable PROC_FS, I can still select the NFS client
>>> options, even NFSv4.
>>
>> OK. I'm confused, then:
>>
>> NFS_V4 selects RPCSEC_GSS_KRB5
>> RPCSEC_GSS_KRB5 (after this patch) depends on PROC_FS
>>
>> What am I missing?
>
> It appears that the "select RPCSEC_GSS_KRB5" is a no-op if
> RPCSEC_GSS_KRB5 hasn't met it's forward dependencies.

Oh, OK, I didn't understand that.

However, this means that it's no longer possible to build a V4 client
with krb5 support without also including PROC_FS. So I think we can't
apply this as is.

--b.