2016-07-07 11:02:34

by Benjamin Coddington

[permalink] [raw]
Subject: [PATCH 1/2] nfsd: allow SCSI layout support without block layout

We shouldn't have to configure both NFSD_BLOCKLAYOUT and NFSD_SCSILAYOUT if
all we want are SCSI layouts on the server, so define the xfs export
operations for either configuration.

Signed-off-by: Benjamin Coddington <[email protected]>
---
fs/xfs/xfs_export.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_export.c b/fs/xfs/xfs_export.c
index a1b2dd828b9d..b08a5541f292 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,
-#ifdef CONFIG_NFSD_BLOCKLAYOUT
+#if defined(CONFIG_NFSD_BLOCKLAYOUT) || defined(CONFIG_NFSD_SCSILAYOUT)
.get_uuid = xfs_fs_get_uuid,
.map_blocks = xfs_fs_map_blocks,
.commit_blocks = xfs_fs_commit_blocks,
--
2.5.5



2016-07-07 11:02:34

by Benjamin Coddington

[permalink] [raw]
Subject: [PATCH 2/2] xfs: abstract block export operations from nfsd layouts

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 <[email protected]>
---
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


2016-07-07 15:43:42

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 2/2] xfs: abstract block export operations from nfsd layouts

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 <[email protected]>

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 <[email protected]>
> ---
> 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

2016-07-07 22:38:45

by Dave Chinner

[permalink] [raw]
Subject: Re: [PATCH 2/2] xfs: abstract block export operations from nfsd layouts

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 <[email protected]>
> ---
> 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.
--
Dave Chinner
[email protected]

2016-07-08 13:23:14

by Benjamin Coddington

[permalink] [raw]
Subject: Re: [PATCH 2/2] abstract block export operations from nfsd layouts

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 <[email protected]>
>> ---
>> 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

2016-07-08 23:30:20

by Dave Chinner

[permalink] [raw]
Subject: Re: [PATCH 2/2] abstract block export operations from nfsd layouts

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 <[email protected]>
> >> ---
> >> 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.