diff mbox series

[v5,04/17] xfs: mount-api - add fs parameter description

Message ID 157062063161.32346.15357252773768069084.stgit@fedora-28 (mailing list archive)
State Deferred, archived
Headers show
Series xfs: mount API patch series | expand

Commit Message

Ian Kent Oct. 9, 2019, 11:30 a.m. UTC 
The new mount-api uses an array of struct fs_parameter_spec for
parameter parsing, create this table populated with the xfs mount
parameters.

The new mount-api table definition is wider than the token based
parameter table and interleaving the option description comments
between each table line is much less readable than adding them to
the end of each table entry. So add the option description comment
to each entry line even though it causes quite a few of the entries
to be longer than 80 characters.

Signed-off-by: Ian Kent <raven@themaw.net>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_super.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

Comments

Christoph Hellwig Oct. 9, 2019, 2:48 p.m. UTC | #1 
On Wed, Oct 09, 2019 at 07:30:31PM +0800, Ian Kent wrote:
> +static const struct fs_parameter_spec xfs_param_specs[] = {
> + fsparam_u32	("logbufs",    Opt_logbufs),   /* number of XFS log buffers */
> + fsparam_string ("logbsize",   Opt_logbsize),  /* size of XFS log buffers */

This has really weird indentation, and a couple overly long lines below.
I'm also not really sure the comments are all that useful here vs in
the actual parser.

Why not:

static const struct fs_parameter_spec xfs_param_specs[] = {
	fsparam_u32("logbufs", Opt_logbufs),
	fsparam_string("logbsize", Opt_logbsize),
	..
};

?
Christoph Hellwig Oct. 9, 2019, 2:53 p.m. UTC | #2 
On Wed, Oct 09, 2019 at 07:30:31PM +0800, Ian Kent wrote:
> +static const struct fs_parameter_description xfs_fs_parameters = {
> +	.name		= "XFS",
> +	.specs		= xfs_param_specs,
> +};

As this isn't used anywhere it will create a compiler warning and not
bisect cleanly.
Ian Kent Oct. 10, 2019, 12:56 a.m. UTC | #3 
Hi Christoph,

On Wed, 2019-10-09 at 07:48 -0700, Christoph Hellwig wrote:
> On Wed, Oct 09, 2019 at 07:30:31PM +0800, Ian Kent wrote:
> > +static const struct fs_parameter_spec xfs_param_specs[] = {
> > + fsparam_u32	("logbufs",    Opt_logbufs),   /* number of XFS
> > log buffers */
> > + fsparam_string ("logbsize",   Opt_logbsize),  /* size of XFS log
> > buffers */
> 
> This has really weird indentation, and a couple overly long lines
> below.
> I'm also not really sure the comments are all that useful here vs in
> the actual parser.
> 
> Why not:
> 
> static const struct fs_parameter_spec xfs_param_specs[] = {
> 	fsparam_u32("logbufs", Opt_logbufs),
> 	fsparam_string("logbsize", Opt_logbsize),
> 	..
> };
> 
> ?

The indentation is purely an effort to preserve the comments.

I originally interleaved them in the structure declaration but
that was even uglier and naturally attracted comments.

I can't remember now if there was a specific request to preserve
the comments.

You suggestion is to add these comments to the case handling in
xfs_parse_param(), correct?

I can do that if there are no other suggestions.

Ian
Christoph Hellwig Oct. 10, 2019, 6:39 a.m. UTC | #4 
On Thu, Oct 10, 2019 at 08:56:31AM +0800, Ian Kent wrote:
> You suggestion is to add these comments to the case handling in
> xfs_parse_param(), correct?

If there is a strong case for keeping them at all.  Some of them
might be so obvious that we can just drop them I think.
Ian Kent Oct. 10, 2019, 8:31 a.m. UTC | #5 
On Wed, 2019-10-09 at 23:39 -0700, Christoph Hellwig wrote:
> On Thu, Oct 10, 2019 at 08:56:31AM +0800, Ian Kent wrote:
> > You suggestion is to add these comments to the case handling in
> > xfs_parse_param(), correct?
> 
> If there is a strong case for keeping them at all.  Some of them
> might be so obvious that we can just drop them I think.

I'll try and make sensible judgements when I change it and see
who complains, ;)

Ian
diff mbox series

Patch

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 1010097354a6..9c1ce3d70c08 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -38,6 +38,8 @@ 
 
 #include <linux/magic.h>
 #include <linux/parser.h>
+#include <linux/fs_context.h>
+#include <linux/fs_parser.h>
 
 static const struct super_operations xfs_super_operations;
 struct bio_set xfs_ioend_bioset;
@@ -108,6 +110,54 @@  static const match_table_t tokens = {
 	{Opt_err,	NULL},
 };
 
+static const struct fs_parameter_spec xfs_param_specs[] = {
+ fsparam_u32	("logbufs",    Opt_logbufs),   /* number of XFS log buffers */
+ fsparam_string ("logbsize",   Opt_logbsize),  /* size of XFS log buffers */
+ fsparam_string ("logdev",     Opt_logdev),    /* log device */
+ fsparam_string ("rtdev",      Opt_rtdev),     /* realtime I/O device */
+ fsparam_flag	("wsync",      Opt_wsync),     /* safe-mode nfs compatible mount */
+ fsparam_flag	("noalign",    Opt_noalign),   /* turn off stripe alignment */
+ fsparam_flag	("swalloc",    Opt_swalloc),   /* turn on stripe width allocation */
+ fsparam_u32	("sunit",      Opt_sunit),     /* data volume stripe unit */
+ fsparam_u32	("swidth",     Opt_swidth),    /* data volume stripe width */
+ fsparam_flag	("nouuid",     Opt_nouuid),    /* ignore filesystem UUID */
+ fsparam_flag   ("grpid",      Opt_grpid),     /* group-ID from parent directory */
+ fsparam_flag   ("nogrpid",    Opt_nogrpid),   /* no group-ID from parent directory */
+ fsparam_flag	("bsdgroups",  Opt_bsdgroups), /* group-ID from parent directory */
+ fsparam_flag	("sysvgroups", Opt_sysvgroups),/* group-ID from current process */
+ fsparam_string ("allocsize",  Opt_allocsize), /* preferred allocation size */
+ fsparam_flag	("norecovery", Opt_norecovery),/* don't run XFS recovery */
+ fsparam_flag	("inode64",    Opt_inode64),   /* inodes can be allocated anywhere */
+ fsparam_flag	("inode32",    Opt_inode32),   /* inode allocation limited to XFS_MAXINUMBER_32 */
+ fsparam_flag   ("ikeep",      Opt_ikeep),     /* do not free empty inode clusters */
+ fsparam_flag   ("noikeep",    Opt_noikeep),   /* free empty inode clusters */
+ fsparam_flag   ("largeio",    Opt_largeio),   /* report large I/O sizes in stat() */
+ fsparam_flag   ("nolargeio",  Opt_nolargeio), /* do not report large I/O sizes in stat() */
+ fsparam_flag   ("attr2",      Opt_attr2),     /* do use attr2 attribute format */
+ fsparam_flag   ("noattr2",    Opt_noattr2),   /* do not use attr2 attribute format */
+ fsparam_flag	("filestreams",Opt_filestreams), /* use filestreams allocator */
+ fsparam_flag   ("quota",      Opt_quota),     /* disk quotas (user) */
+ fsparam_flag   ("noquota",    Opt_noquota),   /* no quotas */
+ fsparam_flag	("usrquota",   Opt_usrquota),  /* user quota enabled */
+ fsparam_flag	("grpquota",   Opt_grpquota),  /* group quota enabled */
+ fsparam_flag	("prjquota",   Opt_prjquota),  /* project quota enabled */
+ fsparam_flag	("uquota",     Opt_uquota),    /* user quota (IRIX variant) */
+ fsparam_flag	("gquota",     Opt_gquota),    /* group quota (IRIX variant) */
+ fsparam_flag	("pquota",     Opt_pquota),    /* project quota (IRIX variant) */
+ fsparam_flag	("uqnoenforce",Opt_uqnoenforce), /* user quota limit enforcement */
+ fsparam_flag	("gqnoenforce",Opt_gqnoenforce), /* group quota limit enforcement */
+ fsparam_flag	("pqnoenforce",Opt_pqnoenforce), /* project quota limit enforcement */
+ fsparam_flag	("qnoenforce", Opt_qnoenforce),  /* same as uqnoenforce */
+ fsparam_flag   ("discard",    Opt_discard),   /* Discard unused blocks */
+ fsparam_flag   ("nodiscard",  Opt_nodiscard), /* Do not discard unused blocks */
+ fsparam_flag	("dax",	       Opt_dax),       /* Enable direct access to bdev pages */
+ {}
+};
+
+static const struct fs_parameter_description xfs_fs_parameters = {
+	.name		= "XFS",
+	.specs		= xfs_param_specs,
+};
 
 STATIC int
 suffix_kstrtoint(const substring_t *s, unsigned int base, int *res)