| Message ID | 1380220824-13047-1-git-send-email-bhalevy@primarydata.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Sep 26, 2013 at 02:40:24PM -0400, Benny Halevy wrote: > From: Benny Halevy <bhalevy@panasas.com> > > Verify whether the server and file system support the given layout type. > > [was pnfsd: Streamline error code checking for non-pnfs filesystems] > Signed-off-by: Dean Hildebrand <seattleplus@gmail.com> > [pnfsd: Add super block to layout_type()] > Signed-off-by: Marc Eshel <eshel@almaden.ibm.com> > [pnfsd: Fix order of ops in nfsd4_layout_verify] > Signed-off-by: Dean Hildebrand <dhildeb@us.ibm.com> > [pnfsd: convert generic code to use new pnfs api] > [pnfsd: define pnfs_export_operations] > [pnfsd: obliterate old vfs api] > Signed-off-by: Benny Halevy <bhalevy@panasas.com> > [pnfsd: layout verify all layout types] > Signed-off-by: Andy Adamson <andros@netapp.com> > [pnfsd: tone nfsd4_layout_verify printk down to dprintk] > Signed-off-by: Benny Halevy <bhalevy@panasas.com> > [pnfsd: check ex_pnfs in nfsd4_verify_layout] > Signed-off-by: Andy Adamson <andros@netapp.com> > [pnfsd: handle s_pnfs_op==NULL] > [pnfsd: verify export option only if svc_export is present] > Signed-off-by: Benny Halevy <bhalevy@panasas.com> > Signed-off-by: Benny Halevy <bhalevy@primarydata.com> > --- > fs/nfsd/export.c | 6 ++++++ > fs/nfsd/nfs4proc.c | 39 +++++++++++++++++++++++++++++++++++++++ > fs/nfsd/pnfsd.h | 2 ++ > include/linux/nfsd/nfsd4_pnfs.h | 5 ++++- > 4 files changed, 51 insertions(+), 1 deletion(-) > > diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c > index 7730dfd..d803414 100644 > --- a/fs/nfsd/export.c > +++ b/fs/nfsd/export.c > @@ -376,6 +376,12 @@ static int check_export(struct inode *inode, int *flags, unsigned char *uuid) > return -EINVAL; > } > > + if (inode->i_sb->s_pnfs_op && > + !inode->i_sb->s_pnfs_op->layout_type) { > + dprintk("exp_export: export of invalid fs pnfs export ops.\n"); > + return -EINVAL; > + } > + If you haven't already done it you may want to look at modifying nfs-utils/utils/exportfs/exportfs.c:test_export() to add the pnfs option when appropriate so the error can be returned at exportfs time. > return 0; > > } > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c > index 419572f..576b635 100644 > --- a/fs/nfsd/nfs4proc.c > +++ b/fs/nfsd/nfs4proc.c > @@ -41,6 +41,7 @@ > #include "vfs.h" > #include "current_stateid.h" > #include "netns.h" > +#include "pnfsd.h" > > #ifdef CONFIG_NFSD_V4_SECURITY_LABEL > #include <linux/security.h> > @@ -1109,6 +1110,44 @@ static int fill_in_write_vector(struct kvec *vec, struct nfsd4_write *write) > return status == nfserr_same ? nfs_ok : status; > } > > +#if defined(CONFIG_PNFSD) > +static __be32 > +nfsd4_layout_verify(struct super_block *sb, struct svc_export *exp, > + unsigned int layout_type) > +{ > + int status, type; > + > + /* check to see if pNFS is supported. */ > + status = nfserr_layoutunavailable; > + if (exp && exp->ex_pnfs == 0) { Can this really be called with exp == NULL? If so don't you want to fail that as well? > + dprintk("%s: Underlying file system " > + "is not exported over pNFS\n", __func__); > + goto out; > + } > + if (!sb->s_pnfs_op || !sb->s_pnfs_op->layout_type) { > + dprintk("%s: Underlying file system " > + "does not support pNFS\n", __func__); > + goto out; > + } > + > + type = sb->s_pnfs_op->layout_type(sb); > + > + /* check to see if requested layout type is supported. */ > + status = nfserr_unknown_layouttype; > + if (!type) > + dprintk("BUG: %s: layout_type 0 is reserved and must not be " > + "used by filesystem\n", __func__); > + else if (type != layout_type) > + dprintk("%s: requested layout type %d " > + "does not match supported type %d\n", > + __func__, layout_type, type); > + else > + status = nfs_ok; > +out: > + return status; > +} > +#endif /* CONFIG_PNFSD */ > + > /* > * NULL call. > */ > diff --git a/fs/nfsd/pnfsd.h b/fs/nfsd/pnfsd.h > index 65fb57e..7c46791 100644 > --- a/fs/nfsd/pnfsd.h > +++ b/fs/nfsd/pnfsd.h > @@ -34,4 +34,6 @@ > #ifndef LINUX_NFSD_PNFSD_H > #define LINUX_NFSD_PNFSD_H > > +#include <linux/nfsd/nfsd4_pnfs.h> > + > #endif /* LINUX_NFSD_PNFSD_H */ > diff --git a/include/linux/nfsd/nfsd4_pnfs.h b/include/linux/nfsd/nfsd4_pnfs.h > index ff6613e..d44669e 100644 > --- a/include/linux/nfsd/nfsd4_pnfs.h > +++ b/include/linux/nfsd/nfsd4_pnfs.h > @@ -34,6 +34,8 @@ > #ifndef _LINUX_NFSD_NFSD4_PNFS_H > #define _LINUX_NFSD_NFSD4_PNFS_H > > +#include <linux/exportfs.h> > + > /* > * pNFS export operations vector. > * > @@ -45,7 +47,8 @@ > * All other methods are optional and can be set to NULL if not implemented. > */ > struct pnfs_export_operations { > - /* stub */ > + /* Returns the supported pnfs_layouttype4. */ > + int (*layout_type) (struct super_block *); > }; > > #endif /* _LINUX_NFSD_NFSD4_PNFS_H */ > -- > 1.8.3.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2013-09-27 17:44, J. Bruce Fields wrote: > On Thu, Sep 26, 2013 at 02:40:24PM -0400, Benny Halevy wrote: >> From: Benny Halevy <bhalevy@panasas.com> >> >> Verify whether the server and file system support the given layout type. >> >> [was pnfsd: Streamline error code checking for non-pnfs filesystems] >> Signed-off-by: Dean Hildebrand <seattleplus@gmail.com> >> [pnfsd: Add super block to layout_type()] >> Signed-off-by: Marc Eshel <eshel@almaden.ibm.com> >> [pnfsd: Fix order of ops in nfsd4_layout_verify] >> Signed-off-by: Dean Hildebrand <dhildeb@us.ibm.com> >> [pnfsd: convert generic code to use new pnfs api] >> [pnfsd: define pnfs_export_operations] >> [pnfsd: obliterate old vfs api] >> Signed-off-by: Benny Halevy <bhalevy@panasas.com> >> [pnfsd: layout verify all layout types] >> Signed-off-by: Andy Adamson <andros@netapp.com> >> [pnfsd: tone nfsd4_layout_verify printk down to dprintk] >> Signed-off-by: Benny Halevy <bhalevy@panasas.com> >> [pnfsd: check ex_pnfs in nfsd4_verify_layout] >> Signed-off-by: Andy Adamson <andros@netapp.com> >> [pnfsd: handle s_pnfs_op==NULL] >> [pnfsd: verify export option only if svc_export is present] >> Signed-off-by: Benny Halevy <bhalevy@panasas.com> >> Signed-off-by: Benny Halevy <bhalevy@primarydata.com> >> --- >> fs/nfsd/export.c | 6 ++++++ >> fs/nfsd/nfs4proc.c | 39 +++++++++++++++++++++++++++++++++++++++ >> fs/nfsd/pnfsd.h | 2 ++ >> include/linux/nfsd/nfsd4_pnfs.h | 5 ++++- >> 4 files changed, 51 insertions(+), 1 deletion(-) >> >> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c >> index 7730dfd..d803414 100644 >> --- a/fs/nfsd/export.c >> +++ b/fs/nfsd/export.c >> @@ -376,6 +376,12 @@ static int check_export(struct inode *inode, int *flags, unsigned char *uuid) >> return -EINVAL; >> } >> >> + if (inode->i_sb->s_pnfs_op && >> + !inode->i_sb->s_pnfs_op->layout_type) { >> + dprintk("exp_export: export of invalid fs pnfs export ops.\n"); >> + return -EINVAL; >> + } >> + > > If you haven't already done it you may want to look at modifying > nfs-utils/utils/exportfs/exportfs.c:test_export() to add the pnfs option > when appropriate so the error can be returned at exportfs time. Hmm, I'm not sure I follow your proposal. In fs/nfsd/exportf.c:check_export() we check whether i_sb->s_export_op and respectively, i_sb->s_pnfs_op support the required export methods. How would we know in utils/exportfs when is appropriate to add the pnfs option? Benny > >> return 0; >> >> } >> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c >> index 419572f..576b635 100644 >> --- a/fs/nfsd/nfs4proc.c >> +++ b/fs/nfsd/nfs4proc.c >> @@ -41,6 +41,7 @@ >> #include "vfs.h" >> #include "current_stateid.h" >> #include "netns.h" >> +#include "pnfsd.h" >> >> #ifdef CONFIG_NFSD_V4_SECURITY_LABEL >> #include <linux/security.h> >> @@ -1109,6 +1110,44 @@ static int fill_in_write_vector(struct kvec *vec, struct nfsd4_write *write) >> return status == nfserr_same ? nfs_ok : status; >> } >> >> +#if defined(CONFIG_PNFSD) >> +static __be32 >> +nfsd4_layout_verify(struct super_block *sb, struct svc_export *exp, >> + unsigned int layout_type) >> +{ >> + int status, type; >> + >> + /* check to see if pNFS is supported. */ >> + status = nfserr_layoutunavailable; >> + if (exp && exp->ex_pnfs == 0) { > > Can this really be called with exp == NULL? If so don't you want to > fail that as well? It is called with exp == NULL from nfsd4_getdevinfo where it shouldn't cause an error return. Benny > >> + dprintk("%s: Underlying file system " >> + "is not exported over pNFS\n", __func__); >> + goto out; >> + } >> + if (!sb->s_pnfs_op || !sb->s_pnfs_op->layout_type) { >> + dprintk("%s: Underlying file system " >> + "does not support pNFS\n", __func__); >> + goto out; >> + } >> + >> + type = sb->s_pnfs_op->layout_type(sb); >> + >> + /* check to see if requested layout type is supported. */ >> + status = nfserr_unknown_layouttype; >> + if (!type) >> + dprintk("BUG: %s: layout_type 0 is reserved and must not be " >> + "used by filesystem\n", __func__); >> + else if (type != layout_type) >> + dprintk("%s: requested layout type %d " >> + "does not match supported type %d\n", >> + __func__, layout_type, type); >> + else >> + status = nfs_ok; >> +out: >> + return status; >> +} >> +#endif /* CONFIG_PNFSD */ >> + >> /* >> * NULL call. >> */ >> diff --git a/fs/nfsd/pnfsd.h b/fs/nfsd/pnfsd.h >> index 65fb57e..7c46791 100644 >> --- a/fs/nfsd/pnfsd.h >> +++ b/fs/nfsd/pnfsd.h >> @@ -34,4 +34,6 @@ >> #ifndef LINUX_NFSD_PNFSD_H >> #define LINUX_NFSD_PNFSD_H >> >> +#include <linux/nfsd/nfsd4_pnfs.h> >> + >> #endif /* LINUX_NFSD_PNFSD_H */ >> diff --git a/include/linux/nfsd/nfsd4_pnfs.h b/include/linux/nfsd/nfsd4_pnfs.h >> index ff6613e..d44669e 100644 >> --- a/include/linux/nfsd/nfsd4_pnfs.h >> +++ b/include/linux/nfsd/nfsd4_pnfs.h >> @@ -34,6 +34,8 @@ >> #ifndef _LINUX_NFSD_NFSD4_PNFS_H >> #define _LINUX_NFSD_NFSD4_PNFS_H >> >> +#include <linux/exportfs.h> >> + >> /* >> * pNFS export operations vector. >> * >> @@ -45,7 +47,8 @@ >> * All other methods are optional and can be set to NULL if not implemented. >> */ >> struct pnfs_export_operations { >> - /* stub */ >> + /* Returns the supported pnfs_layouttype4. */ >> + int (*layout_type) (struct super_block *); >> }; >> >> #endif /* _LINUX_NFSD_NFSD4_PNFS_H */ >> -- >> 1.8.3.1 >> -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Sep 29, 2013 at 02:16:40PM +0300, Benny Halevy wrote: > On 2013-09-27 17:44, J. Bruce Fields wrote: > > On Thu, Sep 26, 2013 at 02:40:24PM -0400, Benny Halevy wrote: > >> From: Benny Halevy <bhalevy@panasas.com> > >> > >> Verify whether the server and file system support the given layout type. > >> > >> [was pnfsd: Streamline error code checking for non-pnfs filesystems] > >> Signed-off-by: Dean Hildebrand <seattleplus@gmail.com> > >> [pnfsd: Add super block to layout_type()] > >> Signed-off-by: Marc Eshel <eshel@almaden.ibm.com> > >> [pnfsd: Fix order of ops in nfsd4_layout_verify] > >> Signed-off-by: Dean Hildebrand <dhildeb@us.ibm.com> > >> [pnfsd: convert generic code to use new pnfs api] > >> [pnfsd: define pnfs_export_operations] > >> [pnfsd: obliterate old vfs api] > >> Signed-off-by: Benny Halevy <bhalevy@panasas.com> > >> [pnfsd: layout verify all layout types] > >> Signed-off-by: Andy Adamson <andros@netapp.com> > >> [pnfsd: tone nfsd4_layout_verify printk down to dprintk] > >> Signed-off-by: Benny Halevy <bhalevy@panasas.com> > >> [pnfsd: check ex_pnfs in nfsd4_verify_layout] > >> Signed-off-by: Andy Adamson <andros@netapp.com> > >> [pnfsd: handle s_pnfs_op==NULL] > >> [pnfsd: verify export option only if svc_export is present] > >> Signed-off-by: Benny Halevy <bhalevy@panasas.com> > >> Signed-off-by: Benny Halevy <bhalevy@primarydata.com> > >> --- > >> fs/nfsd/export.c | 6 ++++++ > >> fs/nfsd/nfs4proc.c | 39 +++++++++++++++++++++++++++++++++++++++ > >> fs/nfsd/pnfsd.h | 2 ++ > >> include/linux/nfsd/nfsd4_pnfs.h | 5 ++++- > >> 4 files changed, 51 insertions(+), 1 deletion(-) > >> > >> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c > >> index 7730dfd..d803414 100644 > >> --- a/fs/nfsd/export.c > >> +++ b/fs/nfsd/export.c > >> @@ -376,6 +376,12 @@ static int check_export(struct inode *inode, int *flags, unsigned char *uuid) > >> return -EINVAL; > >> } > >> > >> + if (inode->i_sb->s_pnfs_op && > >> + !inode->i_sb->s_pnfs_op->layout_type) { > >> + dprintk("exp_export: export of invalid fs pnfs export ops.\n"); > >> + return -EINVAL; > >> + } > >> + > > > > If you haven't already done it you may want to look at modifying > > nfs-utils/utils/exportfs/exportfs.c:test_export() to add the pnfs option > > when appropriate so the error can be returned at exportfs time. > > Hmm, I'm not sure I follow your proposal. > In fs/nfsd/exportf.c:check_export() we check whether i_sb->s_export_op and > respectively, i_sb->s_pnfs_op support the required export methods. > How would we know in utils/exportfs when is appropriate to add the pnfs option? At exportfs time, if somebody requests pnfs, but this filesystem doesn't support that, you probably want to return an error. The way you'd do that would be by passing that pnfs option to nfs-utils/utils/exportfs/exportfs.c:test_export() and including it on the test-export it passes to the kernel. That test export will then succeed or fail depending on whether the filesystem supports pnfs or not. Otherwise the failure in check_export() above won't be noticed until it's too late to give helpful feedback to the user. --b. > > Benny > > > > >> return 0; > >> > >> } > >> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c > >> index 419572f..576b635 100644 > >> --- a/fs/nfsd/nfs4proc.c > >> +++ b/fs/nfsd/nfs4proc.c > >> @@ -41,6 +41,7 @@ > >> #include "vfs.h" > >> #include "current_stateid.h" > >> #include "netns.h" > >> +#include "pnfsd.h" > >> > >> #ifdef CONFIG_NFSD_V4_SECURITY_LABEL > >> #include <linux/security.h> > >> @@ -1109,6 +1110,44 @@ static int fill_in_write_vector(struct kvec *vec, struct nfsd4_write *write) > >> return status == nfserr_same ? nfs_ok : status; > >> } > >> > >> +#if defined(CONFIG_PNFSD) > >> +static __be32 > >> +nfsd4_layout_verify(struct super_block *sb, struct svc_export *exp, > >> + unsigned int layout_type) > >> +{ > >> + int status, type; > >> + > >> + /* check to see if pNFS is supported. */ > >> + status = nfserr_layoutunavailable; > >> + if (exp && exp->ex_pnfs == 0) { > > > > Can this really be called with exp == NULL? If so don't you want to > > fail that as well? > > It is called with exp == NULL from nfsd4_getdevinfo where it shouldn't > cause an error return. > > Benny > > > > >> + dprintk("%s: Underlying file system " > >> + "is not exported over pNFS\n", __func__); > >> + goto out; > >> + } > >> + if (!sb->s_pnfs_op || !sb->s_pnfs_op->layout_type) { > >> + dprintk("%s: Underlying file system " > >> + "does not support pNFS\n", __func__); > >> + goto out; > >> + } > >> + > >> + type = sb->s_pnfs_op->layout_type(sb); > >> + > >> + /* check to see if requested layout type is supported. */ > >> + status = nfserr_unknown_layouttype; > >> + if (!type) > >> + dprintk("BUG: %s: layout_type 0 is reserved and must not be " > >> + "used by filesystem\n", __func__); > >> + else if (type != layout_type) > >> + dprintk("%s: requested layout type %d " > >> + "does not match supported type %d\n", > >> + __func__, layout_type, type); > >> + else > >> + status = nfs_ok; > >> +out: > >> + return status; > >> +} > >> +#endif /* CONFIG_PNFSD */ > >> + > >> /* > >> * NULL call. > >> */ > >> diff --git a/fs/nfsd/pnfsd.h b/fs/nfsd/pnfsd.h > >> index 65fb57e..7c46791 100644 > >> --- a/fs/nfsd/pnfsd.h > >> +++ b/fs/nfsd/pnfsd.h > >> @@ -34,4 +34,6 @@ > >> #ifndef LINUX_NFSD_PNFSD_H > >> #define LINUX_NFSD_PNFSD_H > >> > >> +#include <linux/nfsd/nfsd4_pnfs.h> > >> + > >> #endif /* LINUX_NFSD_PNFSD_H */ > >> diff --git a/include/linux/nfsd/nfsd4_pnfs.h b/include/linux/nfsd/nfsd4_pnfs.h > >> index ff6613e..d44669e 100644 > >> --- a/include/linux/nfsd/nfsd4_pnfs.h > >> +++ b/include/linux/nfsd/nfsd4_pnfs.h > >> @@ -34,6 +34,8 @@ > >> #ifndef _LINUX_NFSD_NFSD4_PNFS_H > >> #define _LINUX_NFSD_NFSD4_PNFS_H > >> > >> +#include <linux/exportfs.h> > >> + > >> /* > >> * pNFS export operations vector. > >> * > >> @@ -45,7 +47,8 @@ > >> * All other methods are optional and can be set to NULL if not implemented. > >> */ > >> struct pnfs_export_operations { > >> - /* stub */ > >> + /* Returns the supported pnfs_layouttype4. */ > >> + int (*layout_type) (struct super_block *); > >> }; > >> > >> #endif /* _LINUX_NFSD_NFSD4_PNFS_H */ > >> -- > >> 1.8.3.1 > >> -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Sep 29, 2013 at 02:16:40PM +0300, Benny Halevy wrote: > On 2013-09-27 17:44, J. Bruce Fields wrote: > > On Thu, Sep 26, 2013 at 02:40:24PM -0400, Benny Halevy wrote: > >> return 0; > >> > >> } > >> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c > >> index 419572f..576b635 100644 > >> --- a/fs/nfsd/nfs4proc.c > >> +++ b/fs/nfsd/nfs4proc.c > >> @@ -41,6 +41,7 @@ > >> #include "vfs.h" > >> #include "current_stateid.h" > >> #include "netns.h" > >> +#include "pnfsd.h" > >> > >> #ifdef CONFIG_NFSD_V4_SECURITY_LABEL > >> #include <linux/security.h> > >> @@ -1109,6 +1110,44 @@ static int fill_in_write_vector(struct kvec *vec, struct nfsd4_write *write) > >> return status == nfserr_same ? nfs_ok : status; > >> } > >> > >> +#if defined(CONFIG_PNFSD) > >> +static __be32 > >> +nfsd4_layout_verify(struct super_block *sb, struct svc_export *exp, > >> + unsigned int layout_type) > >> +{ > >> + int status, type; > >> + > >> + /* check to see if pNFS is supported. */ > >> + status = nfserr_layoutunavailable; > >> + if (exp && exp->ex_pnfs == 0) { > > > > Can this really be called with exp == NULL? If so don't you want to > > fail that as well? > > It is called with exp == NULL from nfsd4_getdevinfo where it shouldn't > cause an error return. Looking through the sbid code, this is making me uncomfortable. Among other things this allows you to perform getdeviceinfo even against filesystems that aren't exported. Maybe that's not awful, but. Also the sbid hash holds all these pointers to superblocks, but does it take a reference to them anywhere? I can't see it, in which case there's a crash waiting to happen here if one of them is unmounted. I'm inclined to think these should be referencing exports, not superblocks. --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2013-10-01 23:38, J. Bruce Fields wrote: > On Sun, Sep 29, 2013 at 02:16:40PM +0300, Benny Halevy wrote: >> On 2013-09-27 17:44, J. Bruce Fields wrote: >>> On Thu, Sep 26, 2013 at 02:40:24PM -0400, Benny Halevy wrote: >>>> From: Benny Halevy <bhalevy@panasas.com> >>>> >>>> Verify whether the server and file system support the given layout type. >>>> >>>> [was pnfsd: Streamline error code checking for non-pnfs filesystems] >>>> Signed-off-by: Dean Hildebrand <seattleplus@gmail.com> >>>> [pnfsd: Add super block to layout_type()] >>>> Signed-off-by: Marc Eshel <eshel@almaden.ibm.com> >>>> [pnfsd: Fix order of ops in nfsd4_layout_verify] >>>> Signed-off-by: Dean Hildebrand <dhildeb@us.ibm.com> >>>> [pnfsd: convert generic code to use new pnfs api] >>>> [pnfsd: define pnfs_export_operations] >>>> [pnfsd: obliterate old vfs api] >>>> Signed-off-by: Benny Halevy <bhalevy@panasas.com> >>>> [pnfsd: layout verify all layout types] >>>> Signed-off-by: Andy Adamson <andros@netapp.com> >>>> [pnfsd: tone nfsd4_layout_verify printk down to dprintk] >>>> Signed-off-by: Benny Halevy <bhalevy@panasas.com> >>>> [pnfsd: check ex_pnfs in nfsd4_verify_layout] >>>> Signed-off-by: Andy Adamson <andros@netapp.com> >>>> [pnfsd: handle s_pnfs_op==NULL] >>>> [pnfsd: verify export option only if svc_export is present] >>>> Signed-off-by: Benny Halevy <bhalevy@panasas.com> >>>> Signed-off-by: Benny Halevy <bhalevy@primarydata.com> >>>> --- >>>> fs/nfsd/export.c | 6 ++++++ >>>> fs/nfsd/nfs4proc.c | 39 +++++++++++++++++++++++++++++++++++++++ >>>> fs/nfsd/pnfsd.h | 2 ++ >>>> include/linux/nfsd/nfsd4_pnfs.h | 5 ++++- >>>> 4 files changed, 51 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c >>>> index 7730dfd..d803414 100644 >>>> --- a/fs/nfsd/export.c >>>> +++ b/fs/nfsd/export.c >>>> @@ -376,6 +376,12 @@ static int check_export(struct inode *inode, int *flags, unsigned char *uuid) >>>> return -EINVAL; >>>> } >>>> >>>> + if (inode->i_sb->s_pnfs_op && >>>> + !inode->i_sb->s_pnfs_op->layout_type) { >>>> + dprintk("exp_export: export of invalid fs pnfs export ops.\n"); >>>> + return -EINVAL; >>>> + } >>>> + >>> >>> If you haven't already done it you may want to look at modifying >>> nfs-utils/utils/exportfs/exportfs.c:test_export() to add the pnfs option >>> when appropriate so the error can be returned at exportfs time. >> >> Hmm, I'm not sure I follow your proposal. >> In fs/nfsd/exportf.c:check_export() we check whether i_sb->s_export_op and >> respectively, i_sb->s_pnfs_op support the required export methods. >> How would we know in utils/exportfs when is appropriate to add the pnfs option? > > At exportfs time, if somebody requests pnfs, but this filesystem doesn't > support that, you probably want to return an error. > > The way you'd do that would be by passing that pnfs option to > nfs-utils/utils/exportfs/exportfs.c:test_export() and including it on > the test-export it passes to the kernel. That test export will then > succeed or fail depending on whether the filesystem supports pnfs or > not. > > Otherwise the failure in check_export() above won't be noticed until > it's too late to give helpful feedback to the user. I see. Thanks. > > --b. > >> >> Benny >> >>> >>>> return 0; >>>> >>>> } >>>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c >>>> index 419572f..576b635 100644 >>>> --- a/fs/nfsd/nfs4proc.c >>>> +++ b/fs/nfsd/nfs4proc.c >>>> @@ -41,6 +41,7 @@ >>>> #include "vfs.h" >>>> #include "current_stateid.h" >>>> #include "netns.h" >>>> +#include "pnfsd.h" >>>> >>>> #ifdef CONFIG_NFSD_V4_SECURITY_LABEL >>>> #include <linux/security.h> >>>> @@ -1109,6 +1110,44 @@ static int fill_in_write_vector(struct kvec *vec, struct nfsd4_write *write) >>>> return status == nfserr_same ? nfs_ok : status; >>>> } >>>> >>>> +#if defined(CONFIG_PNFSD) >>>> +static __be32 >>>> +nfsd4_layout_verify(struct super_block *sb, struct svc_export *exp, >>>> + unsigned int layout_type) >>>> +{ >>>> + int status, type; >>>> + >>>> + /* check to see if pNFS is supported. */ >>>> + status = nfserr_layoutunavailable; >>>> + if (exp && exp->ex_pnfs == 0) { >>> >>> Can this really be called with exp == NULL? If so don't you want to >>> fail that as well? >> >> It is called with exp == NULL from nfsd4_getdevinfo where it shouldn't >> cause an error return. >> >> Benny >> >>> >>>> + dprintk("%s: Underlying file system " >>>> + "is not exported over pNFS\n", __func__); >>>> + goto out; >>>> + } >>>> + if (!sb->s_pnfs_op || !sb->s_pnfs_op->layout_type) { >>>> + dprintk("%s: Underlying file system " >>>> + "does not support pNFS\n", __func__); >>>> + goto out; >>>> + } >>>> + >>>> + type = sb->s_pnfs_op->layout_type(sb); >>>> + >>>> + /* check to see if requested layout type is supported. */ >>>> + status = nfserr_unknown_layouttype; >>>> + if (!type) >>>> + dprintk("BUG: %s: layout_type 0 is reserved and must not be " >>>> + "used by filesystem\n", __func__); >>>> + else if (type != layout_type) >>>> + dprintk("%s: requested layout type %d " >>>> + "does not match supported type %d\n", >>>> + __func__, layout_type, type); >>>> + else >>>> + status = nfs_ok; >>>> +out: >>>> + return status; >>>> +} >>>> +#endif /* CONFIG_PNFSD */ >>>> + >>>> /* >>>> * NULL call. >>>> */ >>>> diff --git a/fs/nfsd/pnfsd.h b/fs/nfsd/pnfsd.h >>>> index 65fb57e..7c46791 100644 >>>> --- a/fs/nfsd/pnfsd.h >>>> +++ b/fs/nfsd/pnfsd.h >>>> @@ -34,4 +34,6 @@ >>>> #ifndef LINUX_NFSD_PNFSD_H >>>> #define LINUX_NFSD_PNFSD_H >>>> >>>> +#include <linux/nfsd/nfsd4_pnfs.h> >>>> + >>>> #endif /* LINUX_NFSD_PNFSD_H */ >>>> diff --git a/include/linux/nfsd/nfsd4_pnfs.h b/include/linux/nfsd/nfsd4_pnfs.h >>>> index ff6613e..d44669e 100644 >>>> --- a/include/linux/nfsd/nfsd4_pnfs.h >>>> +++ b/include/linux/nfsd/nfsd4_pnfs.h >>>> @@ -34,6 +34,8 @@ >>>> #ifndef _LINUX_NFSD_NFSD4_PNFS_H >>>> #define _LINUX_NFSD_NFSD4_PNFS_H >>>> >>>> +#include <linux/exportfs.h> >>>> + >>>> /* >>>> * pNFS export operations vector. >>>> * >>>> @@ -45,7 +47,8 @@ >>>> * All other methods are optional and can be set to NULL if not implemented. >>>> */ >>>> struct pnfs_export_operations { >>>> - /* stub */ >>>> + /* Returns the supported pnfs_layouttype4. */ >>>> + int (*layout_type) (struct super_block *); >>>> }; >>>> >>>> #endif /* _LINUX_NFSD_NFSD4_PNFS_H */ >>>> -- >>>> 1.8.3.1 >>>> > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c index 7730dfd..d803414 100644 --- a/fs/nfsd/export.c +++ b/fs/nfsd/export.c @@ -376,6 +376,12 @@ static int check_export(struct inode *inode, int *flags, unsigned char *uuid) return -EINVAL; } + if (inode->i_sb->s_pnfs_op && + !inode->i_sb->s_pnfs_op->layout_type) { + dprintk("exp_export: export of invalid fs pnfs export ops.\n"); + return -EINVAL; + } + return 0; } diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 419572f..576b635 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -41,6 +41,7 @@ #include "vfs.h" #include "current_stateid.h" #include "netns.h" +#include "pnfsd.h" #ifdef CONFIG_NFSD_V4_SECURITY_LABEL #include <linux/security.h> @@ -1109,6 +1110,44 @@ static int fill_in_write_vector(struct kvec *vec, struct nfsd4_write *write) return status == nfserr_same ? nfs_ok : status; } +#if defined(CONFIG_PNFSD) +static __be32 +nfsd4_layout_verify(struct super_block *sb, struct svc_export *exp, + unsigned int layout_type) +{ + int status, type; + + /* check to see if pNFS is supported. */ + status = nfserr_layoutunavailable; + if (exp && exp->ex_pnfs == 0) { + dprintk("%s: Underlying file system " + "is not exported over pNFS\n", __func__); + goto out; + } + if (!sb->s_pnfs_op || !sb->s_pnfs_op->layout_type) { + dprintk("%s: Underlying file system " + "does not support pNFS\n", __func__); + goto out; + } + + type = sb->s_pnfs_op->layout_type(sb); + + /* check to see if requested layout type is supported. */ + status = nfserr_unknown_layouttype; + if (!type) + dprintk("BUG: %s: layout_type 0 is reserved and must not be " + "used by filesystem\n", __func__); + else if (type != layout_type) + dprintk("%s: requested layout type %d " + "does not match supported type %d\n", + __func__, layout_type, type); + else + status = nfs_ok; +out: + return status; +} +#endif /* CONFIG_PNFSD */ + /* * NULL call. */ diff --git a/fs/nfsd/pnfsd.h b/fs/nfsd/pnfsd.h index 65fb57e..7c46791 100644 --- a/fs/nfsd/pnfsd.h +++ b/fs/nfsd/pnfsd.h @@ -34,4 +34,6 @@ #ifndef LINUX_NFSD_PNFSD_H #define LINUX_NFSD_PNFSD_H +#include <linux/nfsd/nfsd4_pnfs.h> + #endif /* LINUX_NFSD_PNFSD_H */ diff --git a/include/linux/nfsd/nfsd4_pnfs.h b/include/linux/nfsd/nfsd4_pnfs.h index ff6613e..d44669e 100644 --- a/include/linux/nfsd/nfsd4_pnfs.h +++ b/include/linux/nfsd/nfsd4_pnfs.h @@ -34,6 +34,8 @@ #ifndef _LINUX_NFSD_NFSD4_PNFS_H #define _LINUX_NFSD_NFSD4_PNFS_H +#include <linux/exportfs.h> + /* * pNFS export operations vector. * @@ -45,7 +47,8 @@ * All other methods are optional and can be set to NULL if not implemented. */ struct pnfs_export_operations { - /* stub */ + /* Returns the supported pnfs_layouttype4. */ + int (*layout_type) (struct super_block *); }; #endif /* _LINUX_NFSD_NFSD4_PNFS_H */