diff mbox

[2/2] xfs: abstract block export operations from nfsd layouts

Message ID 073be10a55e5e952adbfd320abcce075fb3958ae.1467889001.git.bcodding@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Benjamin Coddington July 7, 2016, 11:02 a.m. UTC
Instead of creeping pnfs layout configuration into filesystems, move the
definition of block-based export operations under a more abstract
configuration.

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/Kconfig          | 3 +++
 fs/nfsd/Kconfig     | 2 ++
 fs/xfs/Makefile     | 3 +--
 fs/xfs/xfs_export.c | 2 +-
 fs/xfs/xfs_pnfs.h   | 4 ++--
 5 files changed, 9 insertions(+), 5 deletions(-)

Comments

J. Bruce Fields July 7, 2016, 3:43 p.m. UTC | #1
Fine by me.  Dave, I'm happy to take it through the nfsd tree, or if you
want to take it, feel free to add my

	Acked-by: J. Bruce Fields <bfields@redhat.com>

I'm OK either way.

--b.

On Thu, Jul 07, 2016 at 07:02:32AM -0400, Benjamin Coddington wrote:
> Instead of creeping pnfs layout configuration into filesystems, move the
> definition of block-based export operations under a more abstract
> configuration.
> 
> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
> ---
>  fs/Kconfig          | 3 +++
>  fs/nfsd/Kconfig     | 2 ++
>  fs/xfs/Makefile     | 3 +--
>  fs/xfs/xfs_export.c | 2 +-
>  fs/xfs/xfs_pnfs.h   | 4 ++--
>  5 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/Kconfig b/fs/Kconfig
> index 6725f59c18e6..6e57b4237d72 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -66,6 +66,9 @@ config FS_POSIX_ACL
>  config EXPORTFS
>  	tristate
>  
> +config BLOCK_EXPORT_OPS
> +	bool
> +
>  config FILE_LOCKING
>  	bool "Enable POSIX file locking API" if EXPERT
>  	default y
> diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
> index c9f583d7bac8..fb63f93cd5f1 100644
> --- a/fs/nfsd/Kconfig
> +++ b/fs/nfsd/Kconfig
> @@ -90,6 +90,7 @@ config NFSD_BLOCKLAYOUT
>  	bool "NFSv4.1 server support for pNFS block layouts"
>  	depends on NFSD_V4 && BLOCK
>  	select NFSD_PNFS
> +	select BLOCK_EXPORT_OPS
>  	help
>  	  This option enables support for the exporting pNFS block layouts
>  	  in the kernel's NFS server. The pNFS block layout enables NFS
> @@ -102,6 +103,7 @@ config NFSD_SCSILAYOUT
>  	bool "NFSv4.1 server support for pNFS SCSI layouts"
>  	depends on NFSD_V4 && BLOCK
>  	select NFSD_PNFS
> +	select BLOCK_EXPORT_OPS
>  	help
>  	  This option enables support for the exporting pNFS SCSI layouts
>  	  in the kernel's NFS server. The pNFS SCSI layout enables NFS
> diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
> index 3542d94fddce..9c9f039e2f05 100644
> --- a/fs/xfs/Makefile
> +++ b/fs/xfs/Makefile
> @@ -121,5 +121,4 @@ xfs-$(CONFIG_XFS_RT)		+= xfs_rtalloc.o
>  xfs-$(CONFIG_XFS_POSIX_ACL)	+= xfs_acl.o
>  xfs-$(CONFIG_SYSCTL)		+= xfs_sysctl.o
>  xfs-$(CONFIG_COMPAT)		+= xfs_ioctl32.o
> -xfs-$(CONFIG_NFSD_BLOCKLAYOUT)	+= xfs_pnfs.o
> -xfs-$(CONFIG_NFSD_SCSILAYOUT)	+= xfs_pnfs.o
> +xfs-$(CONFIG_BLOCK_EXPORT_OPS)	+= xfs_pnfs.o
> diff --git a/fs/xfs/xfs_export.c b/fs/xfs/xfs_export.c
> index b08a5541f292..7b1896ef9112 100644
> --- a/fs/xfs/xfs_export.c
> +++ b/fs/xfs/xfs_export.c
> @@ -246,7 +246,7 @@ const struct export_operations xfs_export_operations = {
>  	.fh_to_parent		= xfs_fs_fh_to_parent,
>  	.get_parent		= xfs_fs_get_parent,
>  	.commit_metadata	= xfs_fs_nfs_commit_metadata,
> -#if defined(CONFIG_NFSD_BLOCKLAYOUT) || defined(CONFIG_NFSD_SCSILAYOUT)
> +#ifdef CONFIG_BLOCK_EXPORT_OPS
>  	.get_uuid		= xfs_fs_get_uuid,
>  	.map_blocks		= xfs_fs_map_blocks,
>  	.commit_blocks		= xfs_fs_commit_blocks,
> diff --git a/fs/xfs/xfs_pnfs.h b/fs/xfs/xfs_pnfs.h
> index 93f74853961b..1073f08cd668 100644
> --- a/fs/xfs/xfs_pnfs.h
> +++ b/fs/xfs/xfs_pnfs.h
> @@ -1,7 +1,7 @@
>  #ifndef _XFS_PNFS_H
>  #define _XFS_PNFS_H 1
>  
> -#if defined(CONFIG_NFSD_BLOCKLAYOUT) || defined(CONFIG_NFSD_SCSILAYOUT)
> +#ifdef CONFIG_BLOCK_EXPORT_OPS
>  int xfs_fs_get_uuid(struct super_block *sb, u8 *buf, u32 *len, u64 *offset);
>  int xfs_fs_map_blocks(struct inode *inode, loff_t offset, u64 length,
>  		struct iomap *iomap, bool write, u32 *device_generation);
> @@ -15,5 +15,5 @@ xfs_break_layouts(struct inode *inode, uint *iolock, bool with_imutex)
>  {
>  	return 0;
>  }
> -#endif /* CONFIG_NFSD_PNFS */
> +#endif /* CONFIG_BLOCK_EXPORT_OPS */
>  #endif /* _XFS_PNFS_H */
> -- 
> 2.5.5
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dave Chinner July 7, 2016, 10:38 p.m. UTC | #2
On Thu, Jul 07, 2016 at 07:02:32AM -0400, Benjamin Coddington wrote:
> Instead of creeping pnfs layout configuration into filesystems, move the
> definition of block-based export operations under a more abstract
> configuration.
> 
> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
> ---
>  fs/Kconfig          | 3 +++
>  fs/nfsd/Kconfig     | 2 ++
>  fs/xfs/Makefile     | 3 +--
>  fs/xfs/xfs_export.c | 2 +-
>  fs/xfs/xfs_pnfs.h   | 4 ++--
>  5 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/Kconfig b/fs/Kconfig
> index 6725f59c18e6..6e57b4237d72 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -66,6 +66,9 @@ config FS_POSIX_ACL
>  config EXPORTFS
>  	tristate
>  
> +config BLOCK_EXPORT_OPS
> +	bool
> +

default n, help text?

Also, BLOCK_* prefix config options are for block layer
functionality, hence I suspect this will confuse people because it's
a filesystem config option. EXPORTFS_BLOCK_OPS seems more obvious
and correct to me, as the block mapping ops are part of the exportfs
operations interface....

>  xfs-$(CONFIG_SYSCTL)		+= xfs_sysctl.o
>  xfs-$(CONFIG_COMPAT)		+= xfs_ioctl32.o
> -xfs-$(CONFIG_NFSD_BLOCKLAYOUT)	+= xfs_pnfs.o
> -xfs-$(CONFIG_NFSD_SCSILAYOUT)	+= xfs_pnfs.o
> +xfs-$(CONFIG_BLOCK_EXPORT_OPS)	+= xfs_pnfs.o

Why do we need the first patch to XFS anymore? Just convert it
straight to using CONFIG_EXPORTFS_BLOCK_OPS....


Cheers,

Dave.
Benjamin Coddington July 8, 2016, 1:24 p.m. UTC | #3
On 7 Jul 2016, at 18:38, Dave Chinner wrote:

> On Thu, Jul 07, 2016 at 07:02:32AM -0400, Benjamin Coddington wrote:
>> Instead of creeping pnfs layout configuration into filesystems, move the
>> definition of block-based export operations under a more abstract
>> configuration.
>>
>> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
>> ---
>>  fs/Kconfig          | 3 +++
>>  fs/nfsd/Kconfig     | 2 ++
>>  fs/xfs/Makefile     | 3 +--
>>  fs/xfs/xfs_export.c | 2 +-
>>  fs/xfs/xfs_pnfs.h   | 4 ++--
>>  5 files changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/Kconfig b/fs/Kconfig
>> index 6725f59c18e6..6e57b4237d72 100644
>> --- a/fs/Kconfig
>> +++ b/fs/Kconfig
>> @@ -66,6 +66,9 @@ config FS_POSIX_ACL
>>  config EXPORTFS
>>  	tristate
>>
>> +config BLOCK_EXPORT_OPS
>> +	bool
>> +
>
> default n, help text?

Not set is n, and as it isn't visible or intended to be set by a user, I
left out the help text.  I'll add both for completeness.

> Also, BLOCK_* prefix config options are for block layer
> functionality, hence I suspect this will confuse people because it's
> a filesystem config option. EXPORTFS_BLOCK_OPS seems more obvious
> and correct to me, as the block mapping ops are part of the exportfs
> operations interface....

OK. I agree - that is better.

>>  xfs-$(CONFIG_SYSCTL)		+= xfs_sysctl.o
>>  xfs-$(CONFIG_COMPAT)		+= xfs_ioctl32.o
>> -xfs-$(CONFIG_NFSD_BLOCKLAYOUT)	+= xfs_pnfs.o
>> -xfs-$(CONFIG_NFSD_SCSILAYOUT)	+= xfs_pnfs.o
>> +xfs-$(CONFIG_BLOCK_EXPORT_OPS)	+= xfs_pnfs.o
>
> Why do we need the first patch to XFS anymore? Just convert it
> straight to using CONFIG_EXPORTFS_BLOCK_OPS....

Doing this in a single patch would combine two changes in a single commit:
 - the definition of the extra operations for a config of only SCSI_LAYOUT
 - the addition of CONFIG_EXPORTFS_BLOCK_OPS.

Since the first is the originally intended behavior, and the second fixes it
up, I'll just send it along in a single patch if that's preferred.

Ben
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dave Chinner July 8, 2016, 11:30 p.m. UTC | #4
On Fri, Jul 08, 2016 at 09:24:27AM -0400, Benjamin Coddington wrote:
> On 7 Jul 2016, at 18:38, Dave Chinner wrote:
> 
> > On Thu, Jul 07, 2016 at 07:02:32AM -0400, Benjamin Coddington wrote:
> >> Instead of creeping pnfs layout configuration into filesystems, move the
> >> definition of block-based export operations under a more abstract
> >> configuration.
> >>
> >> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
> >> ---
> >>  fs/Kconfig          | 3 +++
> >>  fs/nfsd/Kconfig     | 2 ++
> >>  fs/xfs/Makefile     | 3 +--
> >>  fs/xfs/xfs_export.c | 2 +-
> >>  fs/xfs/xfs_pnfs.h   | 4 ++--
> >>  5 files changed, 9 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/fs/Kconfig b/fs/Kconfig
> >> index 6725f59c18e6..6e57b4237d72 100644
> >> --- a/fs/Kconfig
> >> +++ b/fs/Kconfig
> >> @@ -66,6 +66,9 @@ config FS_POSIX_ACL
> >>  config EXPORTFS
> >>  	tristate
> >>
> >> +config BLOCK_EXPORT_OPS
> >> +	bool
> >> +
> >
> > default n, help text?
> 
> Not set is n, and as it isn't visible or intended to be set by a user, I
> left out the help text.  I'll add both for completeness.
> 
> > Also, BLOCK_* prefix config options are for block layer
> > functionality, hence I suspect this will confuse people because it's
> > a filesystem config option. EXPORTFS_BLOCK_OPS seems more obvious
> > and correct to me, as the block mapping ops are part of the exportfs
> > operations interface....
> 
> OK. I agree - that is better.
> 
> >>  xfs-$(CONFIG_SYSCTL)		+= xfs_sysctl.o
> >>  xfs-$(CONFIG_COMPAT)		+= xfs_ioctl32.o
> >> -xfs-$(CONFIG_NFSD_BLOCKLAYOUT)	+= xfs_pnfs.o
> >> -xfs-$(CONFIG_NFSD_SCSILAYOUT)	+= xfs_pnfs.o
> >> +xfs-$(CONFIG_BLOCK_EXPORT_OPS)	+= xfs_pnfs.o
> >
> > Why do we need the first patch to XFS anymore? Just convert it
> > straight to using CONFIG_EXPORTFS_BLOCK_OPS....
> 
> Doing this in a single patch would combine two changes in a single commit:
>  - the definition of the extra operations for a config of only SCSI_LAYOUT
>  - the addition of CONFIG_EXPORTFS_BLOCK_OPS.
> 
> Since the first is the originally intended behavior, and the second fixes it
> up, I'll just send it along in a single patch if that's preferred.

From the XFS perspective, the second change makes the first change
completely redundant. We don't need to care how NFS is configured,
all we care about is whether the exportfs block ops need to be
compiled in.

One patch to fix it all is fine by me - it's a simple, obvious
change and it can be put through the NFS tree without causing us any
problems...

Cheers,

Dave.
diff mbox

Patch

diff --git a/fs/Kconfig b/fs/Kconfig
index 6725f59c18e6..6e57b4237d72 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -66,6 +66,9 @@  config FS_POSIX_ACL
 config EXPORTFS
 	tristate
 
+config BLOCK_EXPORT_OPS
+	bool
+
 config FILE_LOCKING
 	bool "Enable POSIX file locking API" if EXPERT
 	default y
diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
index c9f583d7bac8..fb63f93cd5f1 100644
--- a/fs/nfsd/Kconfig
+++ b/fs/nfsd/Kconfig
@@ -90,6 +90,7 @@  config NFSD_BLOCKLAYOUT
 	bool "NFSv4.1 server support for pNFS block layouts"
 	depends on NFSD_V4 && BLOCK
 	select NFSD_PNFS
+	select BLOCK_EXPORT_OPS
 	help
 	  This option enables support for the exporting pNFS block layouts
 	  in the kernel's NFS server. The pNFS block layout enables NFS
@@ -102,6 +103,7 @@  config NFSD_SCSILAYOUT
 	bool "NFSv4.1 server support for pNFS SCSI layouts"
 	depends on NFSD_V4 && BLOCK
 	select NFSD_PNFS
+	select BLOCK_EXPORT_OPS
 	help
 	  This option enables support for the exporting pNFS SCSI layouts
 	  in the kernel's NFS server. The pNFS SCSI layout enables NFS
diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
index 3542d94fddce..9c9f039e2f05 100644
--- a/fs/xfs/Makefile
+++ b/fs/xfs/Makefile
@@ -121,5 +121,4 @@  xfs-$(CONFIG_XFS_RT)		+= xfs_rtalloc.o
 xfs-$(CONFIG_XFS_POSIX_ACL)	+= xfs_acl.o
 xfs-$(CONFIG_SYSCTL)		+= xfs_sysctl.o
 xfs-$(CONFIG_COMPAT)		+= xfs_ioctl32.o
-xfs-$(CONFIG_NFSD_BLOCKLAYOUT)	+= xfs_pnfs.o
-xfs-$(CONFIG_NFSD_SCSILAYOUT)	+= xfs_pnfs.o
+xfs-$(CONFIG_BLOCK_EXPORT_OPS)	+= xfs_pnfs.o
diff --git a/fs/xfs/xfs_export.c b/fs/xfs/xfs_export.c
index b08a5541f292..7b1896ef9112 100644
--- a/fs/xfs/xfs_export.c
+++ b/fs/xfs/xfs_export.c
@@ -246,7 +246,7 @@  const struct export_operations xfs_export_operations = {
 	.fh_to_parent		= xfs_fs_fh_to_parent,
 	.get_parent		= xfs_fs_get_parent,
 	.commit_metadata	= xfs_fs_nfs_commit_metadata,
-#if defined(CONFIG_NFSD_BLOCKLAYOUT) || defined(CONFIG_NFSD_SCSILAYOUT)
+#ifdef CONFIG_BLOCK_EXPORT_OPS
 	.get_uuid		= xfs_fs_get_uuid,
 	.map_blocks		= xfs_fs_map_blocks,
 	.commit_blocks		= xfs_fs_commit_blocks,
diff --git a/fs/xfs/xfs_pnfs.h b/fs/xfs/xfs_pnfs.h
index 93f74853961b..1073f08cd668 100644
--- a/fs/xfs/xfs_pnfs.h
+++ b/fs/xfs/xfs_pnfs.h
@@ -1,7 +1,7 @@ 
 #ifndef _XFS_PNFS_H
 #define _XFS_PNFS_H 1
 
-#if defined(CONFIG_NFSD_BLOCKLAYOUT) || defined(CONFIG_NFSD_SCSILAYOUT)
+#ifdef CONFIG_BLOCK_EXPORT_OPS
 int xfs_fs_get_uuid(struct super_block *sb, u8 *buf, u32 *len, u64 *offset);
 int xfs_fs_map_blocks(struct inode *inode, loff_t offset, u64 length,
 		struct iomap *iomap, bool write, u32 *device_generation);
@@ -15,5 +15,5 @@  xfs_break_layouts(struct inode *inode, uint *iolock, bool with_imutex)
 {
 	return 0;
 }
-#endif /* CONFIG_NFSD_PNFS */
+#endif /* CONFIG_BLOCK_EXPORT_OPS */
 #endif /* _XFS_PNFS_H */