2020-03-05 23:41:10

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH] NFSD: Fix NFS server build errors

[email protected] reports the following build errors arise when
CONFIG_NFSD_V4_2_INTER_SSC is set and the NFS client is not built
into the kernel:

fs/nfsd/nfs4proc.o: In function `nfsd4_do_copy':
nfs4proc.c:(.text+0x23b7): undefined reference to `nfs42_ssc_close'
fs/nfsd/nfs4proc.o: In function `nfsd4_copy':
nfs4proc.c:(.text+0x5d2a): undefined reference to `nfs_sb_deactive'
fs/nfsd/nfs4proc.o: In function `nfsd4_do_async_copy':
nfs4proc.c:(.text+0x61d5): undefined reference to `nfs42_ssc_open'
nfs4proc.c:(.text+0x6389): undefined reference to `nfs_sb_deactive'

The new inter-server copy code invokes client functions. Until the
NFS server has infrastructure to load the appropriate NFS client
modules to handle inter-server copy requests, let's constrain the
way this feature is built.

Reported-by: YueHaibing <[email protected]>
Fixes: ce0887ac96d3 ("NFSD add nfs4 inter ssc to nfsd4_copy")
Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Yue - does this work for you? The dependency is easier for me to
understand.

Bruce and Olga - OK with this temporary solution?

diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
index f368f3215f88..99d2cae91bd6 100644
--- a/fs/nfsd/Kconfig
+++ b/fs/nfsd/Kconfig
@@ -136,7 +136,7 @@ config NFSD_FLEXFILELAYOUT

config NFSD_V4_2_INTER_SSC
bool "NFSv4.2 inter server to server COPY"
- depends on NFSD_V4 && NFS_V4_1 && NFS_V4_2
+ depends on NFSD_V4 && NFS_V4_1 && NFS_V4_2 && NFS_FS=y
help
This option enables support for NFSv4.2 inter server to
server copy where the destination server calls the NFSv4.2


2020-03-06 02:14:59

by Yue Haibing

[permalink] [raw]
Subject: Re: [PATCH] NFSD: Fix NFS server build errors

On 2020/3/6 7:38, Chuck Lever wrote:
> [email protected] reports the following build errors arise when
> CONFIG_NFSD_V4_2_INTER_SSC is set and the NFS client is not built
> into the kernel:
>
> fs/nfsd/nfs4proc.o: In function `nfsd4_do_copy':
> nfs4proc.c:(.text+0x23b7): undefined reference to `nfs42_ssc_close'
> fs/nfsd/nfs4proc.o: In function `nfsd4_copy':
> nfs4proc.c:(.text+0x5d2a): undefined reference to `nfs_sb_deactive'
> fs/nfsd/nfs4proc.o: In function `nfsd4_do_async_copy':
> nfs4proc.c:(.text+0x61d5): undefined reference to `nfs42_ssc_open'
> nfs4proc.c:(.text+0x6389): undefined reference to `nfs_sb_deactive'
>
> The new inter-server copy code invokes client functions. Until the
> NFS server has infrastructure to load the appropriate NFS client
> modules to handle inter-server copy requests, let's constrain the
> way this feature is built.
>
> Reported-by: YueHaibing <[email protected]>
> Fixes: ce0887ac96d3 ("NFSD add nfs4 inter ssc to nfsd4_copy")
> Signed-off-by: Chuck Lever <[email protected]>
> ---
> fs/nfsd/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Yue - does this work for you? The dependency is easier for me to
> understand.

It works for me.

Tested-by: YueHaibing <[email protected]> # build-tested
>
> Bruce and Olga - OK with this temporary solution?
>
> diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
> index f368f3215f88..99d2cae91bd6 100644
> --- a/fs/nfsd/Kconfig
> +++ b/fs/nfsd/Kconfig
> @@ -136,7 +136,7 @@ config NFSD_FLEXFILELAYOUT
>
> config NFSD_V4_2_INTER_SSC
> bool "NFSv4.2 inter server to server COPY"
> - depends on NFSD_V4 && NFS_V4_1 && NFS_V4_2
> + depends on NFSD_V4 && NFS_V4_1 && NFS_V4_2 && NFS_FS=y
> help
> This option enables support for NFSv4.2 inter server to
> server copy where the destination server calls the NFSv4.2
>
>
>

2020-03-06 15:57:12

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH] NFSD: Fix NFS server build errors



> On Mar 5, 2020, at 9:14 PM, Yuehaibing <[email protected]> wrote:
>
> On 2020/3/6 7:38, Chuck Lever wrote:
>> [email protected] reports the following build errors arise when
>> CONFIG_NFSD_V4_2_INTER_SSC is set and the NFS client is not built
>> into the kernel:
>>
>> fs/nfsd/nfs4proc.o: In function `nfsd4_do_copy':
>> nfs4proc.c:(.text+0x23b7): undefined reference to `nfs42_ssc_close'
>> fs/nfsd/nfs4proc.o: In function `nfsd4_copy':
>> nfs4proc.c:(.text+0x5d2a): undefined reference to `nfs_sb_deactive'
>> fs/nfsd/nfs4proc.o: In function `nfsd4_do_async_copy':
>> nfs4proc.c:(.text+0x61d5): undefined reference to `nfs42_ssc_open'
>> nfs4proc.c:(.text+0x6389): undefined reference to `nfs_sb_deactive'
>>
>> The new inter-server copy code invokes client functions. Until the
>> NFS server has infrastructure to load the appropriate NFS client
>> modules to handle inter-server copy requests, let's constrain the
>> way this feature is built.
>>
>> Reported-by: YueHaibing <[email protected]>
>> Fixes: ce0887ac96d3 ("NFSD add nfs4 inter ssc to nfsd4_copy")
>> Signed-off-by: Chuck Lever <[email protected]>
>> ---
>> fs/nfsd/Kconfig | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Yue - does this work for you? The dependency is easier for me to
>> understand.
>
> It works for me.
>
> Tested-by: YueHaibing <[email protected]> # build-tested

Thanks, I've added this tag and pushed to nfsd-5.7-testing.

Bruce and Olga, you can still veto this approach until I push into
linux-next. It will be a couple of weeks at least.


>> Bruce and Olga - OK with this temporary solution?
>>
>> diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
>> index f368f3215f88..99d2cae91bd6 100644
>> --- a/fs/nfsd/Kconfig
>> +++ b/fs/nfsd/Kconfig
>> @@ -136,7 +136,7 @@ config NFSD_FLEXFILELAYOUT
>>
>> config NFSD_V4_2_INTER_SSC
>> bool "NFSv4.2 inter server to server COPY"
>> - depends on NFSD_V4 && NFS_V4_1 && NFS_V4_2
>> + depends on NFSD_V4 && NFS_V4_1 && NFS_V4_2 && NFS_FS=y
>> help
>> This option enables support for NFSv4.2 inter server to
>> server copy where the destination server calls the NFSv4.2

--
Chuck Lever



2020-03-16 16:05:12

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH] NFSD: Fix NFS server build errors

On Fri, 2020-03-06 at 10:56 -0500, Chuck Lever wrote:
> > On Mar 5, 2020, at 9:14 PM, Yuehaibing <[email protected]>
> > wrote:
> >
> > On 2020/3/6 7:38, Chuck Lever wrote:
> > > [email protected] reports the following build errors arise
> > > when
> > > CONFIG_NFSD_V4_2_INTER_SSC is set and the NFS client is not built
> > > into the kernel:
> > >
> > > fs/nfsd/nfs4proc.o: In function `nfsd4_do_copy':
> > > nfs4proc.c:(.text+0x23b7): undefined reference to
> > > `nfs42_ssc_close'
> > > fs/nfsd/nfs4proc.o: In function `nfsd4_copy':
> > > nfs4proc.c:(.text+0x5d2a): undefined reference to
> > > `nfs_sb_deactive'
> > > fs/nfsd/nfs4proc.o: In function `nfsd4_do_async_copy':
> > > nfs4proc.c:(.text+0x61d5): undefined reference to
> > > `nfs42_ssc_open'
> > > nfs4proc.c:(.text+0x6389): undefined reference to
> > > `nfs_sb_deactive'
> > >
> > > The new inter-server copy code invokes client functions. Until
> > > the
> > > NFS server has infrastructure to load the appropriate NFS client
> > > modules to handle inter-server copy requests, let's constrain the
> > > way this feature is built.
> > >
> > > Reported-by: YueHaibing <[email protected]>
> > > Fixes: ce0887ac96d3 ("NFSD add nfs4 inter ssc to nfsd4_copy")
> > > Signed-off-by: Chuck Lever <[email protected]>
> > > ---
> > > fs/nfsd/Kconfig | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > Yue - does this work for you? The dependency is easier for me to
> > > understand.
> >
> > It works for me.
> >
> > Tested-by: YueHaibing <[email protected]> # build-tested
>
> Thanks, I've added this tag and pushed to nfsd-5.7-testing.
>
> Bruce and Olga, you can still veto this approach until I push into
> linux-next. It will be a couple of weeks at least.
>
>
> > > Bruce and Olga - OK with this temporary solution?
> > >
> > > diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
> > > index f368f3215f88..99d2cae91bd6 100644
> > > --- a/fs/nfsd/Kconfig
> > > +++ b/fs/nfsd/Kconfig
> > > @@ -136,7 +136,7 @@ config NFSD_FLEXFILELAYOUT
> > >
> > > config NFSD_V4_2_INTER_SSC
> > > bool "NFSv4.2 inter server to server COPY"
> > > - depends on NFSD_V4 && NFS_V4_1 && NFS_V4_2
> > > + depends on NFSD_V4 && NFS_V4_1 && NFS_V4_2 && NFS_FS=y
> > > help
> > > This option enables support for NFSv4.2 inter server to
> > > server copy where the destination server calls the NFSv4.2
>

I'd like to suggest that we do this differently.

Let's add a structure

struct ssc_client_ops {
struct file *(*open)(struct vfsmount *ss_mnt,
struct nfs_fh *src_fh, nfs4_stateid
*stateid);
void (*close)(struct file *filep);
};

and then allow the client to register that structure in
fs/nfs/nfs_common when it is loaded (and unregister when it is
unloaded). The 'open' and 'close' fields get set to be pointers to the
functions nfs42_ssc_open and nfs42_ssc_close.

We can then add functions in fs/nfs/nfs_common to access the functions
stored in struct ssc_client_ops, and that can be called by the knfsd
server.

This would allow us to keep both the nfs client and server as modules.
Only nfs_common needs to be compiled into the kernel (as is the case
already today).

--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]


2020-03-16 16:30:27

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH] NFSD: Fix NFS server build errors



> On Mar 16, 2020, at 12:04 PM, Trond Myklebust <[email protected]> wrote:
>
> On Fri, 2020-03-06 at 10:56 -0500, Chuck Lever wrote:
>>> On Mar 5, 2020, at 9:14 PM, Yuehaibing <[email protected]>
>>> wrote:
>>>
>>> On 2020/3/6 7:38, Chuck Lever wrote:
>>>> [email protected] reports the following build errors arise
>>>> when
>>>> CONFIG_NFSD_V4_2_INTER_SSC is set and the NFS client is not built
>>>> into the kernel:
>>>>
>>>> fs/nfsd/nfs4proc.o: In function `nfsd4_do_copy':
>>>> nfs4proc.c:(.text+0x23b7): undefined reference to
>>>> `nfs42_ssc_close'
>>>> fs/nfsd/nfs4proc.o: In function `nfsd4_copy':
>>>> nfs4proc.c:(.text+0x5d2a): undefined reference to
>>>> `nfs_sb_deactive'
>>>> fs/nfsd/nfs4proc.o: In function `nfsd4_do_async_copy':
>>>> nfs4proc.c:(.text+0x61d5): undefined reference to
>>>> `nfs42_ssc_open'
>>>> nfs4proc.c:(.text+0x6389): undefined reference to
>>>> `nfs_sb_deactive'
>>>>
>>>> The new inter-server copy code invokes client functions. Until
>>>> the
>>>> NFS server has infrastructure to load the appropriate NFS client
>>>> modules to handle inter-server copy requests, let's constrain the
>>>> way this feature is built.
>>>>
>>>> Reported-by: YueHaibing <[email protected]>
>>>> Fixes: ce0887ac96d3 ("NFSD add nfs4 inter ssc to nfsd4_copy")
>>>> Signed-off-by: Chuck Lever <[email protected]>
>>>> ---
>>>> fs/nfsd/Kconfig | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> Yue - does this work for you? The dependency is easier for me to
>>>> understand.
>>>
>>> It works for me.
>>>
>>> Tested-by: YueHaibing <[email protected]> # build-tested
>>
>> Thanks, I've added this tag and pushed to nfsd-5.7-testing.
>>
>> Bruce and Olga, you can still veto this approach until I push into
>> linux-next. It will be a couple of weeks at least.
>>
>>
>>>> Bruce and Olga - OK with this temporary solution?
>>>>
>>>> diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
>>>> index f368f3215f88..99d2cae91bd6 100644
>>>> --- a/fs/nfsd/Kconfig
>>>> +++ b/fs/nfsd/Kconfig
>>>> @@ -136,7 +136,7 @@ config NFSD_FLEXFILELAYOUT
>>>>
>>>> config NFSD_V4_2_INTER_SSC
>>>> bool "NFSv4.2 inter server to server COPY"
>>>> - depends on NFSD_V4 && NFS_V4_1 && NFS_V4_2
>>>> + depends on NFSD_V4 && NFS_V4_1 && NFS_V4_2 && NFS_FS=y
>>>> help
>>>> This option enables support for NFSv4.2 inter server to
>>>> server copy where the destination server calls the NFSv4.2
>>
>
> I'd like to suggest that we do this differently.
>
> Let's add a structure
>
> struct ssc_client_ops {
> struct file *(*open)(struct vfsmount *ss_mnt,
> struct nfs_fh *src_fh, nfs4_stateid
> *stateid);
> void (*close)(struct file *filep);
> };
>
> and then allow the client to register that structure in
> fs/nfs/nfs_common when it is loaded (and unregister when it is
> unloaded). The 'open' and 'close' fields get set to be pointers to the
> functions nfs42_ssc_open and nfs42_ssc_close.
>
> We can then add functions in fs/nfs/nfs_common to access the functions
> stored in struct ssc_client_ops, and that can be called by the knfsd
> server.
>
> This would allow us to keep both the nfs client and server as modules.
> Only nfs_common needs to be compiled into the kernel (as is the case
> already today).

However perhaps we really want an upcall to do the copy. It could
manage module loading and a temporary mount point with infrastructure
that is already in place, and give a place to hook in policy choices.

No matter which approach is adopted, though, seems to me there is some
discussion and code development that is still needed. Unless someone
can provide such a patch in the next few days, I'd like to continue
with the Kconfig patch for v5.7. It is tested, ready now, and can be
backported easily. And, as this aspect of SSC is brand new, I don't
feel that we need to go to heroic efforts to make everything work
perfectly in v5.7.


--
Chuck Lever