NFSD: Fix NFS server build errors
diff mbox series

Message ID 20200305233433.14530.61315.stgit@klimt.1015granger.net
State New
Headers show
Series
  • NFSD: Fix NFS server build errors
Related show

Commit Message

Chuck Lever March 5, 2020, 11:38 p.m. UTC
yuehaibing@huawei.com 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 <yuehaibing@huawei.com>
Fixes: ce0887ac96d3 ("NFSD add nfs4 inter ssc to nfsd4_copy")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 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?

Comments

YueHaibing March 6, 2020, 2:14 a.m. UTC | #1
On 2020/3/6 7:38, Chuck Lever wrote:
> yuehaibing@huawei.com 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 <yuehaibing@huawei.com>
> Fixes: ce0887ac96d3 ("NFSD add nfs4 inter ssc to nfsd4_copy")
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  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 <yuehaibing@huawei.com> # 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
> 
> 
>
Chuck Lever March 6, 2020, 3:56 p.m. UTC | #2
> On Mar 5, 2020, at 9:14 PM, Yuehaibing <yuehaibing@huawei.com> wrote:
> 
> On 2020/3/6 7:38, Chuck Lever wrote:
>> yuehaibing@huawei.com 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 <yuehaibing@huawei.com>
>> Fixes: ce0887ac96d3 ("NFSD add nfs4 inter ssc to nfsd4_copy")
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>> 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 <yuehaibing@huawei.com> # 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
Trond Myklebust March 16, 2020, 4:04 p.m. UTC | #3
On Fri, 2020-03-06 at 10:56 -0500, Chuck Lever wrote:
> > On Mar 5, 2020, at 9:14 PM, Yuehaibing <yuehaibing@huawei.com>
> > wrote:
> > 
> > On 2020/3/6 7:38, Chuck Lever wrote:
> > > yuehaibing@huawei.com 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 <yuehaibing@huawei.com>
> > > Fixes: ce0887ac96d3 ("NFSD add nfs4 inter ssc to nfsd4_copy")
> > > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > > ---
> > > 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 <yuehaibing@huawei.com> # 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).
Chuck Lever March 16, 2020, 4:23 p.m. UTC | #4
> On Mar 16, 2020, at 12:04 PM, Trond Myklebust <trondmy@hammerspace.com> wrote:
> 
> On Fri, 2020-03-06 at 10:56 -0500, Chuck Lever wrote:
>>> On Mar 5, 2020, at 9:14 PM, Yuehaibing <yuehaibing@huawei.com>
>>> wrote:
>>> 
>>> On 2020/3/6 7:38, Chuck Lever wrote:
>>>> yuehaibing@huawei.com 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 <yuehaibing@huawei.com>
>>>> Fixes: ce0887ac96d3 ("NFSD add nfs4 inter ssc to nfsd4_copy")
>>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>>> ---
>>>> 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 <yuehaibing@huawei.com> # 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

Patch
diff mbox series

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