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

[Dave Chinner <david@fromorbit.com>]

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.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs