Skip to contentSkip to conversation

xfsprogs: add mkfs.xfs configuration file parsing support

PATCH Superseded Luis R. Rodriguez 2 participants 12 messages May 25, 2018 – May 26, 2018

v3 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Luis R. Rodriguez

author
#0 lore raw

v3 cover xfsprogs: add mkfs.xfs configuration file parsing support

This is v3 series should address all prior feedback from the last iteration. If I forgot something please let me know. The changes that went into this v3 and prior are listed below. You can fetch the goods on my kernel.org xfsprogs-dev 20180524-own-parser branch [0].

Lemme know what ya'll think.

Luis

[0] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/xfsprogs-dev.git/log/?h=20180524-own-parser

Changes on v3:

- Prefix errors on the configuration parsing with config file used and exact line.

- Use uint64_t throughout all parsers as requested.

- Parse the CLI early for -c as I originally had implemented this on v1 but now platform_getoptreset() to reset the opts.

- Drop the enum for the source type for the configuration file type, note that since we rely on the stack to set the variables we should then always set the dft.src on main().

- Use getline() for fetching lines as requested

- Add a special iscomment() and isempty() to handle all space and comment parsing. Now when PARSE_INVALID issued we really mean it.

- Embrace latest bikeshed preferences:
config.c, config.h, random function names

- Add man pages to be parsed via configure so that @sysconfigdir@ gets properly parsed

- get_confopts() now iterates over the known sections tab and gives you the right struct right away.

- Add respecification checks for a section.

- Add respecification checks for -c, -c is only allowed once.

- Add --sysconfigdir to debian/rules, note that if you *don't* set --sysconfigdir your man pages will end up with ${prefix}. We could add a secondary target parsing just in case, or we can do some hacks with autoconf for this, but I don't think its worth it. Other packages deal with this by having ./configure always run with --sysconfigdir set (see systemd for instance).

- Reduced the number of declared enums, only enums for the config subparams which are currently supported for parsing are declared.

- mkfs.xfs -c foo now will search for $PWD/foo and if that fails the sysconfigdir/mkfs.xfs.d/foo.

- mkfs.xfs -c ../foo works and so should ./foo, etc.

- The MKFS_XFS_CONFIG variable support was dropped in favor or just allowing the user to specify the full path now.

- Embraces xfsprogs coding style, c'est la vie.

Changes on v2:

- Stayed with our own parser as its the smallest and we're willing to maintain it as its simple and clear.

- Use -c for the configuration file, and drop the "type" nomenclature to avoid confusion on the interwebs.

- Start to split files off

- Duplicate a bit of items as suggested at LSFMM for the configuration parser structures. We can later consolidate if we think its really needed, however we want the freedom to change these as we see fit and most importantly keep the code apart.

- Conflict resolution and validation is managed now by piggy backing off of the idea of using the defaults to instantiate CLI parameters. CLI always overrides.

Luis R. Rodriguez (4):
mkfs: distinguish between struct sb_feat_args and struct cli_params
mkfs: move shared config structs and into their own headers mkfs.xfs: add configuration file parsing support using our own parser debian/rules: use the new sysconfdir configuration setting

 configure.ac                           |   6 +-
 debian/rules                           |   4 +-
 include/builddefs.in                   |   2 +
 man/man5/Makefile                      |   2 +
 man/man5/mkfs.xfs.d.5.in               | 153 ++++++++
 man/man8/Makefile                      |   2 +
 man/man8/{mkfs.xfs.8 => mkfs.xfs.8.in} |  27 ++
 mkfs/Makefile                          |   2 +-
 mkfs/config.c                          | 644 +++++++++++++++++++++++++++++++++
 mkfs/config.h                          |  86 +++++
 mkfs/xfs_mkfs.c                        | 130 ++++---
 11 files changed, 995 insertions(+), 63 deletions(-)
 create mode 100644 man/man5/mkfs.xfs.d.5.in
 rename man/man8/{mkfs.xfs.8 => mkfs.xfs.8.in} (96%)
 create mode 100644 mkfs/config.c
 create mode 100644 mkfs/config.h
Signature · 1 line
2.16.3
author
#1 lore raw

PATCH v3 1/4 mkfs: distinguish between struct sb_feat_args and struct cli_params

The struct sb_feat_args will actually be shared between the code which processes command line options and the configuration file, as such we need to clarify and reflect this clearly in documentation.

Reviewed-by Dave Chinner <[email protected]>
Signed-off-by Luis R. Rodriguez <[email protected]>
---
 mkfs/xfs_mkfs.c | 45 ++++++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 17 deletions(-)
mkfs/xfs_mkfs.c +28 -17
index 3804814b3b8a..95cd6ced13f0 100644
@@ -707,26 +707,18 @@ cli_opt_set(
 }
 
 /*
- * Options configured on the command line.
- *
- * This stores all the specific config parameters the user sets on the command
- * line. We do not use these values directly - they are inputs to the mkfs
- * geometry validation and override any default configuration value we have.
+ * Shared superblock configuration options
  *
- * We don't keep flags to indicate what parameters are set - if we need to check
- * if an option was set on the command line, we check the relevant entry in the
- * option table which records whether it was specified in the .seen and
- * .str_seen variables in the table.
+ * These options provide shared configuration tunables for the filesystem
+ * superblock. There are three possible sources for these options set, each
+ * source can overriding the later source:
  *
- * Some parameters are stored as strings for post-parsing after their dependent
- * options have been resolved (e.g. block size and sector size have been parsed
- * and validated).
+ * 	o built-in defaults
+ * 	o configuration file (XXX)
+ * 	o command line
  *
- * This allows us to check that values have been set without needing separate
- * flags for each value, and hence avoids needing to record and check for each
- * specific option that can set the value later on in the code. In the cases
- * where we don't have a cli_params structure around, the above cli_opt_set()
- * function can be used.
+ * These values are not used directly - they are inputs into the mkfs geometry
+ * validation.
  */
 struct sb_feat_args {
 	int	log_version;

@@ -747,6 +739,25 @@ struct sb_feat_args {
 	bool	nortalign;
 };
 
+/*
+ * Options configured on the command line.
+ *
+ * This stores all the specific config parameters the user sets on the command
+ * line.  We don't keep flags to indicate what parameters are set - if we need
+ * to check if an option was set on the command line, we check the relevant
+ * entry in the option table which records whether it was specified in the
+ * .seen and .str_seen variables in the table.
+ *
+ * Some parameters are stored as strings for post-parsing after their dependent
+ * options have been resolved (e.g. block size and sector size have been parsed
+ * and validated).
+ *
+ * This allows us to check that values have been set without needing separate
+ * flags for each value, and hence avoids needing to record and check for each
+ * specific option that can set the value later on in the code. In the cases
+ * where we don't have a cli_params structure around, the function cli_opt_set()
+ * function can be used.
+ */
 struct cli_params {
 	int	sectorsize;
 	int	blocksize;
Signature · 1 line
2.16.3
Reviewed-by on v3 1/4
#2 lore raw

Re: [PATCH v3 1/4] mkfs: distinguish between struct sb_feat_args and struct cli_params

Quoted 6 lines · Luis R. Rodriguez

The struct sb_feat_args will actually be shared between the code which processes command line options and the configuration file, as such we need to clarify and reflect this clearly in documentation.

Reviewed-by Dave Chinner <[email protected]>
Signed-off-by Luis R. Rodriguez <[email protected]>

Looks ok,

Reviewed-by Darrick J. Wong <[email protected]>

--D

Quoted 77 lines
---
 mkfs/xfs_mkfs.c | 45 ++++++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 17 deletions(-)
mkfs/xfs_mkfs.c +28 -17
index 3804814b3b8a..95cd6ced13f0 100644
@@ -707,26 +707,18 @@ cli_opt_set(
 }
 
 /*
- * Options configured on the command line.
- *
- * This stores all the specific config parameters the user sets on the command
- * line. We do not use these values directly - they are inputs to the mkfs
- * geometry validation and override any default configuration value we have.
+ * Shared superblock configuration options
  *
- * We don't keep flags to indicate what parameters are set - if we need to check
- * if an option was set on the command line, we check the relevant entry in the
- * option table which records whether it was specified in the .seen and
- * .str_seen variables in the table.
+ * These options provide shared configuration tunables for the filesystem
+ * superblock. There are three possible sources for these options set, each
+ * source can overriding the later source:
  *
- * Some parameters are stored as strings for post-parsing after their dependent
- * options have been resolved (e.g. block size and sector size have been parsed
- * and validated).
+ * 	o built-in defaults
+ * 	o configuration file (XXX)
+ * 	o command line
  *
- * This allows us to check that values have been set without needing separate
- * flags for each value, and hence avoids needing to record and check for each
- * specific option that can set the value later on in the code. In the cases
- * where we don't have a cli_params structure around, the above cli_opt_set()
- * function can be used.
+ * These values are not used directly - they are inputs into the mkfs geometry
+ * validation.
  */
 struct sb_feat_args {
 	int	log_version;

@@ -747,6 +739,25 @@ struct sb_feat_args {
 	bool	nortalign;
 };
 
+/*
+ * Options configured on the command line.
+ *
+ * This stores all the specific config parameters the user sets on the command
+ * line.  We don't keep flags to indicate what parameters are set - if we need
+ * to check if an option was set on the command line, we check the relevant
+ * entry in the option table which records whether it was specified in the
+ * .seen and .str_seen variables in the table.
+ *
+ * Some parameters are stored as strings for post-parsing after their dependent
+ * options have been resolved (e.g. block size and sector size have been parsed
+ * and validated).
+ *
+ * This allows us to check that values have been set without needing separate
+ * flags for each value, and hence avoids needing to record and check for each
+ * specific option that can set the value later on in the code. In the cases
+ * where we don't have a cli_params structure around, the function cli_opt_set()
+ * function can be used.
+ */
 struct cli_params {
 	int	sectorsize;
 	int	blocksize;
Signature · 6 lines
2.16.3

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#3 lore raw

PATCH v3 2/4 mkfs: move shared config structs and into their own headers

In reply to Luis R. Rodriguez

Both struct sb_feat_args and struct mkfs_default_params will be shared between CLI processing and the configuration file processing added later, so move these to their own header.

This will help ensure we split things neatly later and also will help ensure the configuration file processing code from the CLI code are kept separate and cannot touch each other's data structures. This also makes it clear what is actually shared between both.

There are no introduced functional changes in this commit and no documentation changes, this is just code shuffling.

Signed-off-by Luis R. Rodriguez <[email protected]>
---
 mkfs/config.h   | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 mkfs/xfs_mkfs.c | 55 +---------------------------------------
 2 files changed, 79 insertions(+), 54 deletions(-)
 create mode 100644 mkfs/config.h
mkfs/config.h new file +78 -0
new file mode 100644
index 000000000000..e5ea968e2d65
@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2000-2005 Silicon Graphics, Inc.
+ * Copyright (c) 2016-2017 Red Hat, Inc.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+#ifndef _XFS_MKFS_CONFIG_H
+#define _XFS_MKFS_CONFIG_H
+
+struct fsxattr;
+
+/*
+ * Shared superblock configuration options
+ *
+ * These options provide shared configuration tunables for the filesystem
+ * superblock. There are three possible sources for these options set, each
+ * source can overriding the later source:
+ *
+ * 	o built-in defaults
+ * 	o configuration file (XXX)
+ * 	o command line
+ *
+ * These values are not used directly - they are inputs into the mkfs geometry
+ * validation.
+ */
+struct sb_feat_args {
+	int	log_version;
+	int	attr_version;
+	int	dir_version;
+	bool	inode_align;		/* XFS_SB_VERSION_ALIGNBIT */
+	bool	nci;			/* XFS_SB_VERSION_BORGBIT */
+	bool	lazy_sb_counters;	/* XFS_SB_VERSION2_LAZYSBCOUNTBIT */
+	bool	parent_pointers;	/* XFS_SB_VERSION2_PARENTBIT */
+	bool	projid32bit;		/* XFS_SB_VERSION2_PROJID32BIT */
+	bool	crcs_enabled;		/* XFS_SB_VERSION2_CRCBIT */
+	bool	dirftype;		/* XFS_SB_VERSION2_FTYPE */
+	bool	finobt;			/* XFS_SB_FEAT_RO_COMPAT_FINOBT */
+	bool	spinodes;		/* XFS_SB_FEAT_INCOMPAT_SPINODES */
+	bool	rmapbt;			/* XFS_SB_FEAT_RO_COMPAT_RMAPBT */
+	bool	reflink;		/* XFS_SB_FEAT_RO_COMPAT_REFLINK */
+	bool	nodalign;
+	bool	nortalign;
+};
+
+/*
+ * Default filesystem features and configuration values
+ *
+ * This structure contains the default mkfs values that are to be used when
+ * a user does not specify the option on the command line. We do not use these
+ * values directly - they are inputs to the mkfs geometry validation and
+ * calculations.
+ */
+struct mkfs_default_params {
+	char	*source;	/* where the defaults came from */
+
+	int	sectorsize;
+	int	blocksize;
+
+	/* feature flags that are set */
+	struct sb_feat_args	sb_feat;
+
+	/* root inode characteristics */
+	struct fsxattr		fsx;
+};
+
+#endif /* _XFS_MKFS_CONFIG_H */
mkfs/xfs_mkfs.c +1 -54

73 lines not on this page · raw message

Signature · 1 line
2.16.3
Reviewed-by on v3 2/4
#4 lore raw

Re: [PATCH v3 2/4] mkfs: move shared config structs and into their own headers

Quoted 84 lines · Luis R. Rodriguez

Both struct sb_feat_args and struct mkfs_default_params will be shared between CLI processing and the configuration file processing added later, so move these to their own header.

This will help ensure we split things neatly later and also will help ensure the configuration file processing code from the CLI code are kept separate and cannot touch each other's data structures. This also makes it clear what is actually shared between both.

There are no introduced functional changes in this commit and no documentation changes, this is just code shuffling.

Signed-off-by Luis R. Rodriguez <[email protected]>
---
 mkfs/config.h   | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 mkfs/xfs_mkfs.c | 55 +---------------------------------------
 2 files changed, 79 insertions(+), 54 deletions(-)
 create mode 100644 mkfs/config.h
mkfs/config.h new file +59 -0
new file mode 100644
index 000000000000..e5ea968e2d65
@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2000-2005 Silicon Graphics, Inc.
+ * Copyright (c) 2016-2017 Red Hat, Inc.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+#ifndef _XFS_MKFS_CONFIG_H
+#define _XFS_MKFS_CONFIG_H
+
+struct fsxattr;
+
+/*
+ * Shared superblock configuration options
+ *
+ * These options provide shared configuration tunables for the filesystem
+ * superblock. There are three possible sources for these options set, each
+ * source can overriding the later source:
+ *
+ * 	o built-in defaults
+ * 	o configuration file (XXX)
+ * 	o command line
+ *
+ * These values are not used directly - they are inputs into the mkfs geometry
+ * validation.
+ */
+struct sb_feat_args {
+	int	log_version;
+	int	attr_version;
+	int	dir_version;
+	bool	inode_align;		/* XFS_SB_VERSION_ALIGNBIT */
+	bool	nci;			/* XFS_SB_VERSION_BORGBIT */
+	bool	lazy_sb_counters;	/* XFS_SB_VERSION2_LAZYSBCOUNTBIT */
+	bool	parent_pointers;	/* XFS_SB_VERSION2_PARENTBIT */
+	bool	projid32bit;		/* XFS_SB_VERSION2_PROJID32BIT */
+	bool	crcs_enabled;		/* XFS_SB_VERSION2_CRCBIT */
+	bool	dirftype;		/* XFS_SB_VERSION2_FTYPE */
+	bool	finobt;			/* XFS_SB_FEAT_RO_COMPAT_FINOBT */
+	bool	spinodes;		/* XFS_SB_FEAT_INCOMPAT_SPINODES */
+	bool	rmapbt;			/* XFS_SB_FEAT_RO_COMPAT_RMAPBT */
+	bool	reflink;		/* XFS_SB_FEAT_RO_COMPAT_REFLINK */
+	bool	nodalign;
+	bool	nortalign;
+};
+
+/*
+ * Default filesystem features and configuration values
+ *

mkfs/config.h line 69

Quoted 10 lines
mkfs/config.h new file +10 -0
+ * This structure contains the default mkfs values that are to be used when
+ * a user does not specify the option on the command line. We do not use these
+ * values directly - they are inputs to the mkfs geometry validation and
+ * calculations.
+ */
+struct mkfs_default_params {
+	char	*source;	/* where the defaults came from */
+
+	int	sectorsize;
+	int	blocksize;

Indentation ^^^ is uneven with the rest of the struct, but otherwise:

Reviewed-by Darrick J. Wong <[email protected]>

--D

Quoted 96 lines
mkfs/config.h new file +9 -0
+
+	/* feature flags that are set */
+	struct sb_feat_args	sb_feat;
+
+	/* root inode characteristics */
+	struct fsxattr		fsx;
+};
+
+#endif /* _XFS_MKFS_CONFIG_H */
mkfs/xfs_mkfs.c +1 -54
index 95cd6ced13f0..217bb972538d 100644
@@ -20,6 +20,7 @@
 #include <ctype.h>
 #include "xfs_multidisk.h"
 #include "libxcmd.h"
+#include "config.h"
 
 
 

@@ -706,39 +707,6 @@ cli_opt_set(
 	       opts->subopt_params[subopt].str_seen;
 }
 
-/*
- * Shared superblock configuration options
- *
- * These options provide shared configuration tunables for the filesystem
- * superblock. There are three possible sources for these options set, each
- * source can overriding the later source:
- *
- * 	o built-in defaults
- * 	o configuration file (XXX)
- * 	o command line
- *
- * These values are not used directly - they are inputs into the mkfs geometry
- * validation.
- */
-struct sb_feat_args {
-	int	log_version;
-	int	attr_version;
-	int	dir_version;
-	bool	inode_align;		/* XFS_SB_VERSION_ALIGNBIT */
-	bool	nci;			/* XFS_SB_VERSION_BORGBIT */
-	bool	lazy_sb_counters;	/* XFS_SB_VERSION2_LAZYSBCOUNTBIT */
-	bool	parent_pointers;	/* XFS_SB_VERSION2_PARENTBIT */
-	bool	projid32bit;		/* XFS_SB_VERSION2_PROJID32BIT */
-	bool	crcs_enabled;		/* XFS_SB_VERSION2_CRCBIT */
-	bool	dirftype;		/* XFS_SB_VERSION2_FTYPE */
-	bool	finobt;			/* XFS_SB_FEAT_RO_COMPAT_FINOBT */
-	bool	spinodes;		/* XFS_SB_FEAT_INCOMPAT_SPINODES */
-	bool	rmapbt;			/* XFS_SB_FEAT_RO_COMPAT_RMAPBT */
-	bool	reflink;		/* XFS_SB_FEAT_RO_COMPAT_REFLINK */
-	bool	nodalign;
-	bool	nortalign;
-};
-
 /*
  * Options configured on the command line.
  *

@@ -850,27 +818,6 @@ struct mkfs_params {
 	struct sb_feat_args	sb_feat;
 };
 
-/*
- * Default filesystem features and configuration values
- *
- * This structure contains the default mkfs values that are to be used when
- * a user does not specify the option on the command line. We do not use these
- * values directly - they are inputs to the mkfs geometry validation and
- * calculations.
- */
-struct mkfs_default_params {
-	char	*source;	/* where the defaults came from */
-
-	int	sectorsize;
-	int	blocksize;
-
-	/* feature flags that are set */
-	struct sb_feat_args	sb_feat;
-
-	/* root inode characteristics */
-	struct fsxattr		fsx;
-};
-
 static void __attribute__((noreturn))
 usage( void )
 {
Signature · 6 lines
2.16.3

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#5 lore raw

Re: [PATCH v3 2/4] mkfs: move shared config structs and into their own headers

Quoted 13 lines · Darrick J. Wong

mkfs/config.h line 69

Quoted 10 lines · Luis R. Rodriguez
mkfs/config.h new file +5 -0
new file mode 100644
index 000000000000..e5ea968e2d65
+struct mkfs_default_params {
+	char	*source;	/* where the defaults came from */
+
+	int	sectorsize;
+	int	blocksize;

Indentation ^^^ is uneven with the rest of the struct, but otherwise:

Bug-compatible :P -- ie it was in the old code IIRC, as all I did was copy and paste on my editor between the two.

Quoted 1 line
Reviewed-by Darrick J. Wong <[email protected]>

Luis

author
#6 lore raw

PATCH v3 3/4 mkfs.xfs: add configuration file parsing support using our own parser

In reply to Luis R. Rodriguez

You may want to stick to specific set of configuration options when creating filesystems with mkfs.xfs -- sometimes due to pure technical reasons, but some other times to ensure systems remain compatible as new features are introduced with older kernels, or if you always want to take advantage of some new feature which would otherwise typically be disruptive.

This adds support for parsing a configuration file to override defaults parameters to be used for mkfs.xfs.

We define an XFS configuration directory, /etc/mkfs.xfs.d/ and allow for different configuration files, if none is specified we look for the default configuration file, /etc/mkfs.xfs.d/default. You can override with -c. For instance, if you specify:

mkfs.xfs -c experimental -f /dev/loop0

The search path for the configuration file will be:

1) $PWD/experimental
2) /etc/mkfs.xfs.d/experimental

Absolute paths are supported, in which case they will be used directly and the mkfs.xfs.d directory is ignored.

To verify what configuration file is used on a system use the typical:

mkfs.xfs -N

There is only a subset of options allowed to be set on the configuration file. The default parameters you can override on a configuration file and their current built-in default settings are:

[data]
noalign=0

[inode]
align=1
projid32bit=1
sparse=0

[log]
lazy-count=1

[metadata]
crc=1
finobt=1
rmapbt=0
reflink=0

[naming]
ftype=1

[rtdev]
noalign=0

Signed-off-by Luis R. Rodriguez <[email protected]>
---
 configure.ac                           |   6 +-
 include/builddefs.in                   |   2 +
 man/man5/Makefile                      |   2 +
 man/man5/mkfs.xfs.d.5.in               | 153 ++++++++
 man/man8/Makefile                      |   2 +
 man/man8/{mkfs.xfs.8 => mkfs.xfs.8.in} |  27 ++
 mkfs/Makefile                          |   2 +-
 mkfs/config.c                          | 644 +++++++++++++++++++++++++++++++++
 mkfs/config.h                          |  10 +-
 mkfs/xfs_mkfs.c                        |  76 +++-
 10 files changed, 910 insertions(+), 14 deletions(-)
 create mode 100644 man/man5/mkfs.xfs.d.5.in
 rename man/man8/{mkfs.xfs.8 => mkfs.xfs.8.in} (96%)
 create mode 100644 mkfs/config.c
configure.ac +5 -1
index 508eefede073..94c5bda725f2 100644
@@ -233,5 +233,9 @@ AC_CHECK_SIZEOF([char *])
 AC_TYPE_UMODE_T
 AC_MANUAL_FORMAT
 
-AC_CONFIG_FILES([include/builddefs])
+AC_CONFIG_FILES([
+	include/builddefs
+	man/man5/mkfs.xfs.d.5
+	man/man8/mkfs.xfs.8
+])
 AC_OUTPUT
include/builddefs.in +2 -0
index 8aac06cf90dc..e1ee9f7ba086 100644
@@ -62,6 +62,7 @@ PKG_LIB_DIR	= @libdir@@libdirsuffix@
 PKG_INC_DIR	= @includedir@/xfs
 DK_INC_DIR	= @includedir@/disk
 PKG_MAN_DIR	= @mandir@
+PKG_ETC_DIR	= @sysconfdir@
 PKG_DOC_DIR	= @datadir@/doc/@pkg_name@
 PKG_LOCALE_DIR	= @datadir@/locale
 

@@ -196,6 +197,7 @@ endif
 
 GCFLAGS = $(DEBUG) \
 	  -DVERSION=\"$(PKG_VERSION)\" -DLOCALEDIR=\"$(PKG_LOCALE_DIR)\"  \
+	  -DROOT_SYSCONFDIR=\"$(PKG_ETC_DIR)\"  \
 	  -DPACKAGE=\"$(PKG_NAME)\" -I$(TOPDIR)/include -I$(TOPDIR)/libxfs
 
 ifeq ($(ENABLE_GETTEXT),yes)
man/man5/Makefile +2 -0
index fe0aef6f016b..0b33122b064e 100644
@@ -19,3 +19,5 @@ install : default
 	$(INSTALL) -m 755 -d $(MAN_DEST)
 	$(INSTALL_MAN)
 install-dev :
+
+LDIRT += mkfs.xfs.d.5
man/man5/mkfs.xfs.d.5.in new file +153 -0

153 lines not on this page · raw message

man/man8/Makefile +2 -0
index 36620da172ae..2a6548079997 100644
@@ -19,3 +19,5 @@ install : default
 	$(INSTALL) -m 755 -d $(MAN_DEST)
 	$(INSTALL_MAN)
 install-dev :
+
+LDIRT += mkfs.xfs.8
man/man8/mkfs.xfs.8.in renamed from man/man8/mkfs.xfs.8 +27 -0
similarity index 96%
rename from man/man8/mkfs.xfs.8
rename to man/man8/mkfs.xfs.8.in
index 4b8c78c37806..81e2753bd2b5 100644
@@ -83,6 +83,24 @@ and
 .B \-l internal \-l size=10m
 are equivalent.
 .PP
+An optional XFS configuration file directory
+.B mkfs.xfs.d (5)
+exists to help fine tune different default parameters which can be used when
+calling
+.B mkfs.xfs (8).
+If present, and if
+.B -c
+is not used, the default configuration file @sysconfigdir@/mkfs.xfs.d/default
+will be used to override system build-in defaults. Refer to mkfs.xfs.d (5)
+for a list of current defaults and further details.
+Command line arguments directly passed to
+.B mkfs.xfs (8)
+will always override parameters set in the configuration file.
+You can override the configuration file used by using the
+.B -c
+parameter, further explained below and in
+.B mkfs.xfs.d (5)
+.PP
 In the descriptions below, sizes are given in sectors, bytes, blocks,
 kilobytes, megabytes, gigabytes, etc.
 Sizes are treated as hexadecimal if prefixed by 0x or 0X,

@@ -123,6 +141,14 @@ Many feature options allow an optional argument of 0 or 1, to explicitly
 disable or enable the functionality.
 .SH OPTIONS
 .TP
+.BI \-c " configuration-file"
+Override the configuration file used. If a relative path is given the search
+path for the configuration file is your current directory and then the
+.B mkfs.xfs.d (5)
+directory. If an absolute path is given it is used directly. For more details
+refer to
+.B mkfs.xfs.d (5)
+.TP
 .BI \-b " block_size_options"
 This option specifies the fundamental block size of the filesystem.
 The valid

@@ -923,6 +949,7 @@ Prints the version number and exits.
 .SH SEE ALSO
 .BR xfs (5),
 .BR mkfs (8),
+.BR mkfs.xfs.d (5),
 .BR mount (8),
 .BR xfs_info (8),
 .BR xfs_admin (8).
mkfs/Makefile +1 -1
index c84f9b6ae63b..c7815b3e106b 100644
@@ -8,7 +8,7 @@ include $(TOPDIR)/include/builddefs
 LTCOMMAND = mkfs.xfs
 
 HFILES =
-CFILES = proto.c xfs_mkfs.c
+CFILES = proto.c xfs_mkfs.c config.c
 
 LLDLIBS += $(LIBXFS) $(LIBXCMD) $(LIBFROG) $(LIBRT) $(LIBPTHREAD) $(LIBBLKID) \
 	$(LIBUUID)
mkfs/config.c new file +644 -0

644 lines not on this page · raw message

mkfs/config.h +9 -1
index e5ea968e2d65..0f429d9b7fd7 100644
@@ -19,6 +19,8 @@
 #ifndef _XFS_MKFS_CONFIG_H
 #define _XFS_MKFS_CONFIG_H
 
+#define MKFS_XFS_CONF_DIR      ROOT_SYSCONFDIR "/mkfs.xfs.d"
+
 struct fsxattr;
 
 /*

@@ -29,7 +31,7 @@ struct fsxattr;
  * source can overriding the later source:
  *
  * 	o built-in defaults
- * 	o configuration file (XXX)
+ * 	o configuration file
  * 	o command line
  *
  * These values are not used directly - they are inputs into the mkfs geometry

@@ -75,4 +77,10 @@ struct mkfs_default_params {
 	struct fsxattr		fsx;
 };
 
+int
+parse_defaults_file(
+	struct mkfs_default_params	*dft,
+	int				default_fd,
+	char				*config_file);
+
 #endif /* _XFS_MKFS_CONFIG_H */
mkfs/xfs_mkfs.c +65 -11

96 lines not on this page · raw message

Signature · 1 line
2.16.3
#7 lore raw

Re: [PATCH v3 3/4] mkfs.xfs: add configuration file parsing support using our own parser

Quoted 78 lines · Luis R. Rodriguez

You may want to stick to specific set of configuration options when creating filesystems with mkfs.xfs -- sometimes due to pure technical reasons, but some other times to ensure systems remain compatible as new features are introduced with older kernels, or if you always want to take advantage of some new feature which would otherwise typically be disruptive.

This adds support for parsing a configuration file to override defaults parameters to be used for mkfs.xfs.

We define an XFS configuration directory, /etc/mkfs.xfs.d/ and allow for different configuration files, if none is specified we look for the default configuration file, /etc/mkfs.xfs.d/default. You can override with -c. For instance, if you specify:

mkfs.xfs -c experimental -f /dev/loop0

The search path for the configuration file will be:

1) $PWD/experimental
2) /etc/mkfs.xfs.d/experimental

Absolute paths are supported, in which case they will be used directly and the mkfs.xfs.d directory is ignored.

To verify what configuration file is used on a system use the typical:

mkfs.xfs -N

There is only a subset of options allowed to be set on the configuration file. The default parameters you can override on a configuration file and their current built-in default settings are:

[data]
noalign=0

[inode]
align=1
projid32bit=1
sparse=0

[log]
lazy-count=1

[metadata]
crc=1
finobt=1
rmapbt=0
reflink=0

[naming]
ftype=1

[rtdev]
noalign=0

Signed-off-by Luis R. Rodriguez <[email protected]>
---
 configure.ac                           |   6 +-
 include/builddefs.in                   |   2 +
 man/man5/Makefile                      |   2 +
 man/man5/mkfs.xfs.d.5.in               | 153 ++++++++
 man/man8/Makefile                      |   2 +
 man/man8/{mkfs.xfs.8 => mkfs.xfs.8.in} |  27 ++
 mkfs/Makefile                          |   2 +-
 mkfs/config.c                          | 644 +++++++++++++++++++++++++++++++++
 mkfs/config.h                          |  10 +-
 mkfs/xfs_mkfs.c                        |  76 +++-
 10 files changed, 910 insertions(+), 14 deletions(-)
 create mode 100644 man/man5/mkfs.xfs.d.5.in
 rename man/man8/{mkfs.xfs.8 => mkfs.xfs.8.in} (96%)
 create mode 100644 mkfs/config.c
configure.ac +0 -0
index 508eefede073..94c5bda725f2 100644

configure.ac AC_CHECK_SIZEOF([char *]) · line 239

Quoted 8 lines
configure.ac +4 -1
@@ -233,5 +233,9 @@ AC_CHECK_SIZEOF([char *])
 AC_TYPE_UMODE_T
 AC_MANUAL_FORMAT
 
-AC_CONFIG_FILES([include/builddefs])
+AC_CONFIG_FILES([
+	include/builddefs
+	man/man5/mkfs.xfs.d.5
+	man/man8/mkfs.xfs.8

Building the manpages as part of configure? That's clever. :)

Quoted 38 lines
configure.ac +1 -0
+])
 AC_OUTPUT
include/builddefs.in +2 -0
index 8aac06cf90dc..e1ee9f7ba086 100644
@@ -62,6 +62,7 @@ PKG_LIB_DIR	= @libdir@@libdirsuffix@
 PKG_INC_DIR	= @includedir@/xfs
 DK_INC_DIR	= @includedir@/disk
 PKG_MAN_DIR	= @mandir@
+PKG_ETC_DIR	= @sysconfdir@
 PKG_DOC_DIR	= @datadir@/doc/@pkg_name@
 PKG_LOCALE_DIR	= @datadir@/locale
 

@@ -196,6 +197,7 @@ endif
 
 GCFLAGS = $(DEBUG) \
 	  -DVERSION=\"$(PKG_VERSION)\" -DLOCALEDIR=\"$(PKG_LOCALE_DIR)\"  \
+	  -DROOT_SYSCONFDIR=\"$(PKG_ETC_DIR)\"  \
 	  -DPACKAGE=\"$(PKG_NAME)\" -I$(TOPDIR)/include -I$(TOPDIR)/libxfs
 
 ifeq ($(ENABLE_GETTEXT),yes)
man/man5/Makefile +2 -0
index fe0aef6f016b..0b33122b064e 100644
@@ -19,3 +19,5 @@ install : default
 	$(INSTALL) -m 755 -d $(MAN_DEST)
 	$(INSTALL_MAN)
 install-dev :
+
+LDIRT += mkfs.xfs.d.5
man/man5/mkfs.xfs.d.5.in new file +0 -0
new file mode 100644
index 000000000000..c8dac58f5c9d

man/man5/mkfs.xfs.d.5.in line 9

Quoted 9 lines
man/man5/mkfs.xfs.d.5.in new file +9 -0
@@ -0,0 +1,153 @@
+.TH mkfs.xfs.d 5
+.SH NAME
+mkfs.xfs.d \- mkfs.xfs configuration directory
+.SH DESCRIPTION
+.B mkfs.xfs (8)
+uses a set of initial default parameters for configuration.
+
+The built-in mkfs defaults are decided by the XFS community. New features are
+only enabled by default when the community consider it stable.  One can

"...consider them stable."

man/man5/mkfs.xfs.d.5.in line 16

Quoted 7 lines
man/man5/mkfs.xfs.d.5.in new file +7 -0
+override these defaults on the
+.B mkfs.xfs (8)
+command line, but there are cases where it is desirable for the distro or the
+system administrator to establish their own supported defaults in a uniform
+manner, regardless of the version of
+.B mkfs.xfs (8)
+used. This may desirable for example on systems with old kernels

"This may be desirable..."

Quoted 19 lines
man/man5/mkfs.xfs.d.5.in new file +19 -0
+where the built-in default
+.B mkfs.xfs (8)
+parameters create a filesystem that is not supported by the old kernel.
+In such situations it would also be unclear what parameters are needed to
+produce a compatible filesystem, having a configuration file present ensures
+that if newer versions of
+.B mkfs.xfs (8)
+are deployed, creating a filesystem will remain compatible. Overriding
+.B mkfs.xfs (8)
+built-in defaults may also be desirable if you have a series of systems with
+different kernels and want to be able to create filesystems which all systems
+are able to support properly.
+.PP
+The XFS configuration directory
+.B mkfs.xfs.d (5)
+can be used as a home to define different configuration files which can be used
+to override the built-in default parameters by
+.B mkfs.xfs (8).
+If the

man/man5/mkfs.xfs.d.5.in line 45

Quoted 10 lines
man/man5/mkfs.xfs.d.5.in new file +10 -0
+.B -c
+parameter is not used, the default configuration file:
+.IP
+.I @sysconfdir@/mkfs.xfs.d/default
+.PP
+will be looked for first and if present will be used to override
+.B mkfs.xfs (8)
+built-in default parameters.
+.PP
+You can override the configuration file by specifying its name when using

"...override the default configuration file...", right?

man/man5/mkfs.xfs.d.5.in line 51

Quoted 6 lines
man/man5/mkfs.xfs.d.5.in new file +6 -0
+.B mkfs.xfs (8)
+by using the
+.B -c
+parameter.
+.PP
+If a relative path without the special character '.' is passed,

"If the path does not start with a '.', the current working directory is searched for the file. If the file is not found there, the mkfs.xfs.d(5) directory is searched for the file." ?

man/man5/mkfs.xfs.d.5.in line 62

Quoted 11 lines
man/man5/mkfs.xfs.d.5.in new file +11 -0
+the configuration file is first looked for in you current working directory;
+if the configuration file is not present in your current working directory
+the configuration file with the name given is looked in the
+.B mkfs.xfs.d (5)
+directory.
+.PP
+If
+.B -c
+is used with relative path with which has a leading '.' character, the given
+path is used directly, so the configuration file will be relative to your
+current directory.

"...relative to the current working directory."

(Trying to be consistent with 'current working directory'.)

man/man5/mkfs.xfs.d.5.in line 67

Quoted 5 lines
man/man5/mkfs.xfs.d.5.in new file +5 -0
+.PP
+If
+.B -c
+is used with an absolute path -- a path with a leading '/' character --
+then the absolute path given is used for the configuration file.

"If the -c argument starts with a '/', it is considered an absolute path, and opened." ?

(dunno about that...)

Quoted 17 lines
man/man5/mkfs.xfs.d.5.in new file +17 -0
+.PP
+For example:
+.IP
+.B mkfs.xfs -c experimental -f /dev/sda1
+.PP
+Will make
+.B mkfs.xfs (8)
+look for the following configuration files and use the first one it finds:
+.IP
+.B $PWD/experimental
+.br
+.B @sysconfdir@/mkfs.xfs.d/experimental
+.PP
+If you used an absolute path, for example:
+.IP
+.B mkfs.xfs -c /tmp/experimental -f /dev/sda1
+.PP

man/man5/mkfs.xfs.d.5.in line 94

Quoted 10 lines
man/man5/mkfs.xfs.d.5.in new file +10 -0
+Then only the configuration file /tmp/experimental will be looked for and
+used if present.
+.PP
+If you use the
+.B -c
+parameter the configuration file must be present and must parse correctly.
+.PP
+Parameters passed to the
+.B mkfs.xfs (8)
+command line always override any defaults set on the configuration file used.

"...set by the configuration file."

Quoted 41 lines
man/man5/mkfs.xfs.d.5.in new file +41 -0
+.PP
+.B mkfs.xfs (8)
+will always describe what configuration file was used, if any
+was used at all. To verify which configuration file would be used prior to
+execution of
+.B mkfs.xfs (8)
+you can use
+.I mkfs.xfs -N.
+.PP
+.SH DEFAULT PARAMETERS
+Default parameters for
+.B mkfs.xfs (8)
+consists of a small subset of the parameters one can set with on the command
+line. Currently all default parameters can only be either enabled or disabled,
+you can set their value to 1 to enable or 0 to disable. Below we list the
+different supported default parameters which can be defined on configuration
+files, along with the current built-in setting.
+.PP
+.BI [data]
+.br
+.BI noalign=0
+.PP
+.BI [inode]
+.br
+.BI align=1
+.br
+.BI projid32bit=1
+.br
+.BI sparse=0
+.PP
+.BI [log]
+.br
+.BI lazy-count=1
+.PP
+.BI [metadata]
+.br
+.BI crc=1
+.br
+.BI finobt=1
+.br
+.BI rmapbt=0

man/man5/mkfs.xfs.d.5.in line 145

Quoted 10 lines
man/man5/mkfs.xfs.d.5.in new file +10 -0
+.br
+.BI reflink=0
+.PP
+.BI [naming]
+.br
+.BI ftype=1
+.PP
+.BI [rtdev]
+.br
+.BI noalign=0

We ought to have a comment in the same place that we define the builtin defaults warning would-be patch authors to ensure that they update this manpage.

Quoted 332 lines
man/man5/mkfs.xfs.d.5.in new file +8 -0
+.PP
+.SH SEE ALSO
+.BR mkfs.xfs (8),
+.BR xfsctl (3),
+.BR xfs_info (8),
+.BR xfs_admin (8),
+.BR xfsdump (8),
+.BR xfsrestore (8).
man/man8/Makefile +2 -0
index 36620da172ae..2a6548079997 100644
@@ -19,3 +19,5 @@ install : default
 	$(INSTALL) -m 755 -d $(MAN_DEST)
 	$(INSTALL_MAN)
 install-dev :
+
+LDIRT += mkfs.xfs.8
man/man8/mkfs.xfs.8.in renamed from man/man8/mkfs.xfs.8 +27 -0
similarity index 96%
rename from man/man8/mkfs.xfs.8
rename to man/man8/mkfs.xfs.8.in
index 4b8c78c37806..81e2753bd2b5 100644
@@ -83,6 +83,24 @@ and
 .B \-l internal \-l size=10m
 are equivalent.
 .PP
+An optional XFS configuration file directory
+.B mkfs.xfs.d (5)
+exists to help fine tune different default parameters which can be used when
+calling
+.B mkfs.xfs (8).
+If present, and if
+.B -c
+is not used, the default configuration file @sysconfigdir@/mkfs.xfs.d/default
+will be used to override system build-in defaults. Refer to mkfs.xfs.d (5)
+for a list of current defaults and further details.
+Command line arguments directly passed to
+.B mkfs.xfs (8)
+will always override parameters set in the configuration file.
+You can override the configuration file used by using the
+.B -c
+parameter, further explained below and in
+.B mkfs.xfs.d (5)
+.PP
 In the descriptions below, sizes are given in sectors, bytes, blocks,
 kilobytes, megabytes, gigabytes, etc.
 Sizes are treated as hexadecimal if prefixed by 0x or 0X,

@@ -123,6 +141,14 @@ Many feature options allow an optional argument of 0 or 1, to explicitly
 disable or enable the functionality.
 .SH OPTIONS
 .TP
+.BI \-c " configuration-file"
+Override the configuration file used. If a relative path is given the search
+path for the configuration file is your current directory and then the
+.B mkfs.xfs.d (5)
+directory. If an absolute path is given it is used directly. For more details
+refer to
+.B mkfs.xfs.d (5)
+.TP
 .BI \-b " block_size_options"
 This option specifies the fundamental block size of the filesystem.
 The valid

@@ -923,6 +949,7 @@ Prints the version number and exits.
 .SH SEE ALSO
 .BR xfs (5),
 .BR mkfs (8),
+.BR mkfs.xfs.d (5),
 .BR mount (8),
 .BR xfs_info (8),
 .BR xfs_admin (8).
mkfs/Makefile +1 -1
index c84f9b6ae63b..c7815b3e106b 100644
@@ -8,7 +8,7 @@ include $(TOPDIR)/include/builddefs
 LTCOMMAND = mkfs.xfs
 
 HFILES =
-CFILES = proto.c xfs_mkfs.c
+CFILES = proto.c xfs_mkfs.c config.c
 
 LLDLIBS += $(LIBXFS) $(LIBXCMD) $(LIBFROG) $(LIBRT) $(LIBPTHREAD) $(LIBBLKID) \
 	$(LIBUUID)
mkfs/config.c new file +240 -0
new file mode 100644
index 000000000000..2d21728b7b62
@@ -0,0 +1,644 @@
+/*
+ * Copyright (c) 2018 Luis R. Rodriguez <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+#include <ctype.h>
+#include <dirent.h>
+#include <fcntl.h>
+
+#include "libxfs.h"
+#include "config.h"
+
+/*
+ * Enums for each configuration option. All these currently match the CLI
+ * parameters for now but this may change later, so we keep all this code
+ * and definitions separate. The rules for configuration parameters may also
+ * differ.
+ *
+ * We only provide definitions for what we currently support parsing.
+ */
+
+enum data_subopts {
+	D_NOALIGN = 0,
+};
+
+enum inode_subopts {
+	I_ALIGN = 0,
+	I_PROJID32BIT,
+	I_SPINODES,
+};
+
+enum log_subopts {
+	L_LAZYSBCNTR = 0,
+};
+
+enum metadata_subopts {
+	M_CRC = 0,
+	M_FINOBT,
+	M_RMAPBT,
+	M_REFLINK,
+};
+
+enum naming_subopts {
+	N_FTYPE = 0,
+};
+
+enum rtdev_subopts {
+	R_NOALIGN = 0,
+};
+
+/* Just define the max options array size manually right now */
+#define MAX_SUBOPTS	4
+
+static int
+data_config_parser(
+	struct mkfs_default_params	*dft,
+	int				psubopt,
+	uint64_t			value)
+{
+	enum data_subopts	subopt = psubopt;
+
+	switch (subopt) {
+	case D_NOALIGN:
+		dft->sb_feat.nodalign = value;
+		return 0;
+	}
+	return EINVAL;
+}
+
+static int
+inode_config_parser(
+	struct mkfs_default_params	*dft,
+	int				psubopt,
+	uint64_t			value)
+{
+	enum inode_subopts	subopt = psubopt;
+
+	switch (subopt) {
+	case I_ALIGN:
+		dft->sb_feat.inode_align = value;
+		return 0;
+	case I_PROJID32BIT:
+		dft->sb_feat.projid32bit = value;
+		return 0;
+	case I_SPINODES:
+		dft->sb_feat.spinodes = value;
+		return 0;
+	}
+	return EINVAL;
+}
+
+static int
+log_config_parser(
+	struct mkfs_default_params	*dft,
+	int				psubopt,
+	uint64_t			value)
+{
+	enum log_subopts	subopt = psubopt;
+
+	switch (subopt) {
+	case L_LAZYSBCNTR:
+		dft->sb_feat.lazy_sb_counters = value;
+		return 0;
+	}
+	return EINVAL;
+}
+
+static int
+metadata_config_parser(
+	struct mkfs_default_params	*dft,
+	int				psubopt,
+	uint64_t			value)
+{
+	enum metadata_subopts	subopt = psubopt;
+
+	switch (subopt) {
+	case M_CRC:
+		dft->sb_feat.crcs_enabled = value;
+		if (dft->sb_feat.crcs_enabled)
+			dft->sb_feat.dirftype = true;
+		return 0;
+	case M_FINOBT:
+		dft->sb_feat.finobt = value;
+		return 0;
+	case M_RMAPBT:
+		dft->sb_feat.rmapbt = value;
+		return 0;
+	case M_REFLINK:
+		dft->sb_feat.reflink = value;
+		return 0;
+	}
+	return EINVAL;
+}
+
+static int
+naming_config_parser(
+	struct mkfs_default_params	*dft,
+	int				psubopt,
+	uint64_t			value)
+{
+	enum naming_subopts	subopt = psubopt;
+
+	switch (subopt) {
+	case N_FTYPE:
+		dft->sb_feat.dirftype = value;
+		return 0;
+	}
+	return EINVAL;
+}
+
+static int
+rtdev_config_parser(
+	struct mkfs_default_params	*dft,
+	int				psubopt,
+	uint64_t			value)
+{
+	enum rtdev_subopts	subopt = psubopt;
+
+	switch (subopt) {
+	case R_NOALIGN:
+		dft->sb_feat.nortalign = value;
+		return 0;
+	}
+	return EINVAL;
+}
+
+struct confopts {
+	const char		*name;
+	const char		*subopts[MAX_SUBOPTS];
+	int			(*parser)(struct mkfs_default_params *dft,
+					  int psubopt, uint64_t value);
+	bool			seen;
+} confopts_tab[] = {
+	{
+		.name = "data",
+		.subopts = {
+			[D_NOALIGN] = "noalign",
+		},
+		.parser = data_config_parser,
+	},
+	{
+		.name = "inode",
+		.subopts = {
+			[I_ALIGN] = "align",
+			[I_PROJID32BIT] = "projid32bit",
+			[I_SPINODES] = "sparse",
+		},
+		.parser = inode_config_parser,
+	},
+	{
+		.name = "log",
+		.subopts = {
+			[L_LAZYSBCNTR] = "lazy-count",
+		},
+		.parser = log_config_parser,
+	},
+	{
+		.name = "naming",
+		.subopts = {
+			[N_FTYPE] = "ftype",
+		},
+		.parser = naming_config_parser,
+	},
+	{
+		.name = "rtdev",
+		.subopts = {
+			[R_NOALIGN] = "noalign",
+		},
+		.parser = rtdev_config_parser,
+	},
+	{
+		.name = "metadata",
+		.subopts = {
+			[M_CRC] = "crc",
+			[M_FINOBT] = "finobt",
+			[M_RMAPBT] = "rmapbt",
+			[M_REFLINK] = "reflink",
+		},
+		.parser = metadata_config_parser,
+	},
+};
+
+static struct confopts *
+get_confopts(
+	const char	*section)
+{
+	unsigned int	i;

mkfs/config.c line 250

Quoted 10 lines
mkfs/config.c new file +10 -0
+	struct confopts	*opts;
+
+	for (i=0; i < ARRAY_SIZE(confopts_tab); i++) {
+		opts = &confopts_tab[i];
+		if (!opts)
+			return NULL;
+		if (strcmp(opts->name, section) == 0) {
+			if (opts->seen) {
+				fprintf(stderr, _("Section '%s' respecified\n"),
+						section);

If I have two [data] sections, will this resuilt in:

# mkfs.xfs -c foo /dev/sda1
Section 'data' respecified
Invalid section on line foo:1 [data]

?

The section isn't invalid, it's just double-specified, so...

mkfs/config.c line 256

Quoted 6 lines
mkfs/config.c new file +6 -0
+				return NULL;
+			}
+			opts->seen = true;
+			return opts;
+		}
+	}

...I'd print the 'Invalid section' error message here.

Quoted 268 lines
mkfs/config.c new file +268 -0
+	return NULL;
+}
+
+enum parse_line_type {
+	PARSE_COMMENT = 0,
+	PARSE_EMPTY,
+	PARSE_SECTION,
+	PARSE_TAG_VALUE,
+	PARSE_INVALID,
+	PARSE_EOF,
+};
+
+static bool
+isempty(
+	const char	*line,
+	ssize_t		linelen)
+{
+	ssize_t		i = 0;
+	char		p;
+
+	while (i != linelen) {
+		p = line[i];
+		i++;
+
+		/* tab or space */
+		if (isblank(p))
+			continue;
+		else
+			return false;
+	}
+
+	return true;
+}
+
+static bool
+iscomment(
+	const char	*line,
+	ssize_t		linelen)
+{
+	ssize_t		i = 0;
+	char		p;
+
+	while (i != linelen) {
+		p = line[i];
+		i++;
+
+		/* tab or space */
+		if (isblank(p))
+			continue;
+
+		if (p == '#')
+			return true;
+
+		return false;
+	}
+
+	return false;
+}
+
+static int
+parse_line_section(
+	const char	*line,
+	char		**tag)
+{
+	return sscanf(line, " [%m[^]]]", tag);
+}
+
+static int
+parse_line_tag_value(
+	const char	*line,
+	char		**tag,
+	uint64_t	*value)
+{
+	return sscanf(line, " %m[^ \t=]"
+		      " = "
+		      "%lu ",
+		      tag, value);
+}
+
+static enum parse_line_type
+parse_get_line_type(
+	const char	*line,
+	ssize_t		linelen,
+	char		**tag,
+	uint64_t	*value)
+{
+	int		ret;
+	uint64_t	u64_value;
+
+	if (isempty(line, linelen))
+		return PARSE_EMPTY;
+
+	if (iscomment(line, linelen))
+		return PARSE_COMMENT;
+
+	ret = parse_line_section(line, tag);
+	if (ret == 1)
+		return  PARSE_SECTION;
+
+	if (ret == EOF)
+		return PARSE_EOF;
+
+	ret = parse_line_tag_value(line, tag, &u64_value);
+	if (ret == 2) {
+		*value = u64_value;
+
+		return PARSE_TAG_VALUE;
+	}
+
+	if (ret == EOF)
+		return PARSE_EOF;
+
+	return PARSE_INVALID;
+}
+
+static int
+parse_config_stream(
+	struct mkfs_default_params	*dft,
+	const char 			*config_file,
+	FILE				*fp)
+{
+	int				ret;
+	char				*line = NULL;
+	ssize_t				linelen;
+	size_t				len = 0, lineno = 0;
+	uint64_t			value;
+	enum parse_line_type		parse_type;
+	struct confopts			*confopt = NULL;
+	int				subopt;
+
+	while ((linelen = getline(&line, &len, fp)) != -1) {
+		char *ignore_value;
+		char *p, *tag = NULL;
+
+		lineno++;
+
+		/*
+		 * tag is allocated for us by scanf(), it must freed only on any
+		 * successful parse of a section or tag-value pair.
+		 */
+		parse_type = parse_get_line_type(line, linelen, &tag, &value);
+
+		switch (parse_type) {
+		case PARSE_EMPTY:
+		case PARSE_COMMENT:
+			/* Nothing tag to free for these */
+			continue;
+		case PARSE_EOF:
+			break;
+		case PARSE_INVALID:
+			ret = EINVAL;
+			fprintf(stderr, _("Invalid line %s:%zu : %s\n"),
+					  config_file, lineno, line);
+			goto out;
+		case PARSE_SECTION:
+			confopt = get_confopts(tag);
+			if (!confopt) {
+				ret = EINVAL;
+				fprintf(stderr, _("Invalid section on line %s:%zu : %s\n"),
+						config_file, lineno, tag);
+				free(tag);
+				goto out;
+			}
+			if (!confopt->subopts) {
+				ret = EINVAL;
+				fprintf(stderr, _("Section not yet supported on line %s:%zu : %s\n"),
+						config_file, lineno, tag);
+				free(tag);
+				goto out;
+			}
+			free(tag);
+			break;
+		case PARSE_TAG_VALUE:
+			if (!confopt) {
+				ret = EINVAL;
+				fprintf(stderr, _("No section specified yet on line %s:%zu : %s\n"),
+						config_file, lineno, line);
+				free(tag);
+				goto out;
+			}
+
+			/*
+			 * We re-use the line buffer allocated by getline(),
+			 * however line must be kept pointing to its original
+			 * value to free it later. A separate pointer is needed
+			 * as getsubopt() will otherwise muck with the value
+			 * passed.
+			 */
+			p = line;
+
+			/*
+			 * Trims white spaces. getsubopt() does not grok
+			 * white space, it would fail otherwise.
+			 */
+			snprintf(p, len, "%s=%lu", tag, value);
+
+			/* Not needed anymore */
+			free(tag);
+
+			/*
+			 * We only use getsubopt() to validate the possible
+			 * subopt, we already parsed the value and its already
+			 * in a more preferred data type.
+			 */
+			subopt = getsubopt(&p, (char **) confopt->subopts,
+					   &ignore_value);
+
+			ret = confopt->parser(dft, subopt, value);
+			if (ret) {
+				fprintf(stderr, _("Error parsine line %s:%zu : %s\n"),
+						config_file, lineno, line);
+				goto out;
+			}
+
+			break;
+		}
+	}
+out:
+	/* We must free even if getline() failed */
+	free(line);
+	return ret;
+}
+
+static const char *conf_paths[] = {
+	".",
+	MKFS_XFS_CONF_DIR,
+};
+
+/*
+ * If the file is not found -1 is returned and errno set. Otherwise
+ * the file descriptor is returned.
+ */
+int
+open_cli_config(
+	char			*cli_config_file,
+	char			**fpath)
+{
+	int			fd, len;
+	char			*final_path = NULL;
+	char			*relative_path= NULL;
+	unsigned int		i;
+
+	if (strlen(cli_config_file) > 2) {
+		if (cli_config_file[0] == '.' && cli_config_file[1] == '/')
+			final_path = cli_config_file;
+		else if (cli_config_file[0] == '.' && cli_config_file[1] == '.')
+			final_path = cli_config_file;
+		else if (cli_config_file[0] == '/')
+			final_path = cli_config_file;
+		else
+			relative_path = cli_config_file;
+	} else if (strlen(cli_config_file) == 1) {
+		if (cli_config_file[0] == '.' || cli_config_file[0] == '/') {
+			errno = EINVAL;
+			return -1;
+		} else
+			relative_path = cli_config_file;
+	}
+
+	if (final_path) {
+		fd = open(final_path, O_RDONLY);
+		if (fd >= 0)
+			memcpy(*fpath, final_path, strlen(final_path));
+		return fd;
+	}
+
+	/* We finally know the path is relative but just to be sure */
+	if (!relative_path) {

mkfs/config.c line 534

Quoted 10 lines
mkfs/config.c new file +10 -0
+		errno = ENXIO;
+		return -1;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(conf_paths); i++) {
+		memset(*fpath, 0, PATH_MAX);
+		/*
+		 * For current directory evaluation, skip concatenating the
+		 * ./ from the file passed. We only concatenate for the other
+		 * paths we look up on.

If conf_paths[0] was "./" then you wouldn't have to special case this, I think.

Quoted 64 lines
mkfs/config.c new file +64 -0
+		 */
+		if (i == 0)
+			memcpy(*fpath, relative_path, strlen(relative_path));
+		else {
+			len = snprintf(*fpath, PATH_MAX, "%s/%s", conf_paths[i],
+				       relative_path);
+			/* Indicates truncation */
+			if (len >= PATH_MAX) {
+				errno = ENAMETOOLONG;
+				return -1;
+			}
+		}
+		fd = open(*fpath, O_RDONLY);
+		if (fd < 0)
+			continue;
+		return fd;
+	}
+
+	errno = ENOENT;
+	return -1;
+}
+
+/*
+ * This is only called *iff* there is a configuration file which we know we
+ * *must* parse.
+ *
+ * If default_fd is set and is a valid file descriptor then the configuration
+ * file passed is the system default configuraiton file, and we already know
+ * it exists. If default_fd is not set we assume we've been passed a
+ * configuration file from the command line and must it must exist, otherwise
+ * we have to error out.
+ */
+int
+parse_defaults_file(
+	struct mkfs_default_params		*dft,
+	int					default_fd,
+	char					*config_file)
+{
+	char			*fpath;
+	int			fd;
+	FILE			*fp;
+	int			ret;
+	struct stat		sp;
+
+	if (strlen(config_file) > PATH_MAX)
+		return ENAMETOOLONG;
+
+	fpath = malloc(PATH_MAX);
+	if (!fpath)
+		return ENOMEM;
+	memset(fpath, 0, PATH_MAX);
+
+	if (default_fd < 0) {
+		fd = open_cli_config(config_file, &fpath);
+		if (fd < 0) {
+			free(fpath);
+			return errno;
+		}
+	} else {
+		fd = default_fd;
+		memcpy(fpath, config_file, strlen(config_file));
+	}
+
+	/*

mkfs/config.c line 608

Quoted 10 lines
mkfs/config.c new file +10 -0
+	 * At this point we know we have a valid file descriptor and have
+	 * figured out the path to the file used on fpath. Get the file stream
+	 * and do a bit of sanity checks before parsing the file.
+	 */
+
+	fp = fdopen(fd, "r");
+	if (!fp) {
+		ret = errno;
+		fprintf(stderr, _("Unable to open stream for config file: %s : %s\n"),
+				fpath, strerror(errno));

perror(fpath); ?

Quoted 158 lines
mkfs/config.c new file +36 -0
+		goto out_close_fd;
+	}
+
+	ret = fstat(fd, &sp);
+	if (ret) {
+		ret = errno;
+		fprintf(stderr, _("Could not fstat() config file: %s: %s\n"),
+			fpath, strerror(errno));
+		goto out;
+	}
+
+	if (S_ISDIR(sp.st_mode)) {
+		ret = EBADF;
+		fprintf(stderr, _("Config file is a directory: %s\n"), fpath);
+		goto out;
+	}
+
+	/* Anything beyond 1 MiB is kind of silly right now */
+	if (sp.st_size > 1 * 1024 * 1024) {
+		ret = E2BIG;
+		goto out;
+	}
+
+	ret = parse_config_stream(dft, config_file, fp);
+	if (ret)
+		goto out;
+
+	printf(_("config-file=%s\n"), fpath);
+
+out:
+	fclose(fp);
+out_close_fd:
+	close(fd);
+	free(fpath);
+	return ret;
+}
mkfs/config.h +9 -1
index e5ea968e2d65..0f429d9b7fd7 100644
@@ -19,6 +19,8 @@
 #ifndef _XFS_MKFS_CONFIG_H
 #define _XFS_MKFS_CONFIG_H
 
+#define MKFS_XFS_CONF_DIR      ROOT_SYSCONFDIR "/mkfs.xfs.d"
+
 struct fsxattr;
 
 /*

@@ -29,7 +31,7 @@ struct fsxattr;
  * source can overriding the later source:
  *
  * 	o built-in defaults
- * 	o configuration file (XXX)
+ * 	o configuration file
  * 	o command line
  *
  * These values are not used directly - they are inputs into the mkfs geometry

@@ -75,4 +77,10 @@ struct mkfs_default_params {
 	struct fsxattr		fsx;
 };
 
+int
+parse_defaults_file(
+	struct mkfs_default_params	*dft,
+	int				default_fd,
+	char				*config_file);
+
 #endif /* _XFS_MKFS_CONFIG_H */
mkfs/xfs_mkfs.c +59 -10
index 217bb972538d..f759e6078ca1 100644
@@ -3777,6 +3777,11 @@ main(
 			.nortalign = false,
 		},
 	};
+	char                    *default_config = MKFS_XFS_CONF_DIR "/default";
+	char			*cli_config_file = NULL;
+	char			*config_file = NULL;
+	int			default_fd = -1;
+	int			ret;
 
 	platform_uuid_generate(&cli.uuid);
 	progname = basename(argv[0]);

@@ -3785,25 +3790,74 @@ main(
 	textdomain(PACKAGE);
 
 	/*
-	 * TODO: Sourcing defaults from a config file
-	 *
 	 * Before anything else, see if there's a config file with different
-	 * defaults. If a file exists in <package location>, read in the new
-	 * default values and overwrite them in the &dft structure. This way the
-	 * new defaults will apply before we parse the CLI, and the CLI will
-	 * still be able to override them. When more than one source is
-	 * implemented, emit a message to indicate where the defaults being
-	 * used came from.
+	 * defaults. If the CLI specified a full path we use and require that.
+	 * If a relative path was provided on the CLI we search the allowed
+	 * search paths for the file. If no config file was specified on the
+	 * CLI we will look for MKFS_XFS_CONF_DIR/default and use that if
+	 * present, however this file is optional.
 	 *
-	 * printf(_("Default configuration sourced from %s\n"), dft.source);
+	 * If a configuration file is found we use it to help overwrite default
+	 * values in the &dft structure. This way the new defaults will apply
+	 * before we parse the CLI, and the user will still be able to override
+	 * them through the CLI.
+	 */
+
+	/*
+	 * Pull config line options from command line
 	 */
+	while ((c = getopt(argc, argv, "b:c:d:i:l:L:m:n:KNp:qr:s:CfV")) != EOF) {
+		switch (c) {
+		case 'c':
+			if (cli_config_file) {
+				fprintf(stderr, _("respecification of configuration not allowed\n"));
+				exit(1);
+			}
+			cli_config_file = optarg;
+			dft.source = _("command line");
+			break;
+		default:
+			continue;
+		}
+	}
+
+	if (cli_config_file)
+		config_file = cli_config_file;
+	else {
+		default_fd = open(default_config, O_RDONLY);
+		if (default_fd >= 0) {
+			dft.source = _("system default configuration file");
+			config_file = default_config;
+		}
+	}
+
+	if (config_file) {
+		/* If default_fd is set it will be closed for us */
+		ret = parse_defaults_file(&dft, default_fd, config_file);
+		if (ret) {
+			fprintf(stderr, _("Could not open %s config file: %s : %s\n"),
+					dft.source, config_file,
+					strerror(ret));
+			exit(1);
+		}
+	}
 
-	/* copy new defaults into CLI parsing structure */
+	printf(_("Default configuration sourced from %s\n"), dft.source);
+
+	/*
+	 * Done parsing defaults now, so memcpy defaults into CLI
+	 * structure, reset getopt and start parsing CLI options
+	 */
 	memcpy(&cli.sb_feat, &dft.sb_feat, sizeof(cli.sb_feat));

mkfs/xfs_mkfs.c main( · line 3860

Quoted 10 lines
mkfs/xfs_mkfs.c +6 -1
 	memcpy(&cli.fsx, &dft.fsx, sizeof(cli.fsx));
 
-	while ((c = getopt(argc, argv, "b:d:i:l:L:m:n:KNp:qr:s:CfV")) != EOF) {
+	platform_getoptreset();
+
+	while ((c = getopt(argc, argv, "b:c:d:i:l:L:m:n:KNp:qr:s:CfV")) != EOF) {
 		switch (c) {
+		case 'c':
+			/* already validated and parsed, ignore */
+			break;

Looks good otherwise.

--D

Quoted 10 lines
mkfs/xfs_mkfs.c +0 -0
 		case 'C':
 		case 'f':
 			force_overwrite = 1;
Signature · 6 lines
2.16.3

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#8 lore raw

Re: [PATCH v3 3/4] mkfs.xfs: add configuration file parsing support using our own parser

I've applied all the recommendations for the man page updates you provided. More below on the code questions.

Quoted 19 lines · Darrick J. Wong
Quoted 5 lines · Luis R. Rodriguez
mkfs/config.c new file +5 -0
+static struct confopts *
+get_confopts(
+	const char	*section)
+{
+	unsigned int	i;

mkfs/config.c line 250

Quoted 10 lines
mkfs/config.c new file +10 -0
+	struct confopts	*opts;
+
+	for (i=0; i < ARRAY_SIZE(confopts_tab); i++) {
+		opts = &confopts_tab[i];
+		if (!opts)
+			return NULL;
+		if (strcmp(opts->name, section) == 0) {
+			if (opts->seen) {
+				fprintf(stderr, _("Section '%s' respecified\n"),
+						section);

If I have two [data] sections, will this resuilt in:

Quoted 5 lines
# mkfs.xfs -c foo /dev/sda1
Section 'data' respecified
Invalid section on line foo:1 [data]

?

Yeah.

Quoted 1 line

The section isn't invalid, it's just double-specified, so...

I've fixed this, now we get:

ergon:~ # mkfs.xfs -f /dev/loop5 -c bar
Section 'metadata' respecified
Error parsing command line config file: bar : Invalid argument

Quoted 8 lines

mkfs/config.c line 256

Quoted 6 lines
mkfs/config.c new file +6 -0
+				return NULL;
+			}
+			opts->seen = true;
+			return opts;
+		}
+	}

...I'd print the 'Invalid section' error message here.

Or move the respecification error message check to the switch statement handlig the section. I went with the later.

Quoted 50 lines
Quoted 50 lines
mkfs/config.c new file +50 -0
+static const char *conf_paths[] = {
+	".",
+	MKFS_XFS_CONF_DIR,
+};
+
+/*
+ * If the file is not found -1 is returned and errno set. Otherwise
+ * the file descriptor is returned.
+ */
+int
+open_cli_config(
+	char			*cli_config_file,
+	char			**fpath)
+{
+	int			fd, len;
+	char			*final_path = NULL;
+	char			*relative_path= NULL;
+	unsigned int		i;
+
+	if (strlen(cli_config_file) > 2) {
+		if (cli_config_file[0] == '.' && cli_config_file[1] == '/')
+			final_path = cli_config_file;
+		else if (cli_config_file[0] == '.' && cli_config_file[1] == '.')
+			final_path = cli_config_file;
+		else if (cli_config_file[0] == '/')
+			final_path = cli_config_file;
+		else
+			relative_path = cli_config_file;
+	} else if (strlen(cli_config_file) == 1) {
+		if (cli_config_file[0] == '.' || cli_config_file[0] == '/') {
+			errno = EINVAL;
+			return -1;
+		} else
+			relative_path = cli_config_file;
+	}
+
+	if (final_path) {
+		fd = open(final_path, O_RDONLY);
+		if (fd >= 0)
+			memcpy(*fpath, final_path, strlen(final_path));
+		return fd;
+	}
+
+	/* We finally know the path is relative but just to be sure */
+	if (!relative_path) {
+		errno = ENXIO;
+		return -1;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(conf_paths); i++) {
Quoted 8 lines

mkfs/config.c line 534

Quoted 5 lines
mkfs/config.c new file +5 -0
+		memset(*fpath, 0, PATH_MAX);
+		/*
+		 * For current directory evaluation, skip concatenating the
+		 * ./ from the file passed. We only concatenate for the other
+		 * paths we look up on.

If conf_paths[0] was "./" then you wouldn't have to special case this, I think.

No, I had tried it, it looks odd still, if the user specified -c foo, the output should show ./foo, not just foo.

Quoted 7 lines

mkfs/config.c line 608

Quoted 5 lines
mkfs/config.c new file +5 -0
+	fp = fdopen(fd, "r");
+	if (!fp) {
+		ret = errno;
+		fprintf(stderr, _("Unable to open stream for config file: %s : %s\n"),
+				fpath, strerror(errno));

perror(fpath); ?

Sure.

Quoted 1 line

Looks good otherwise.

Groovy. I'll wait for others to comment otherwise I can spin up 4th iteration after the weekend.

Luis

#9 lore raw

Re: [PATCH v3 3/4] mkfs.xfs: add configuration file parsing support using our own parser

Quoted 31 lines · Luis R. Rodriguez

I've applied all the recommendations for the man page updates you provided. More below on the code questions.

Quoted 19 lines · Darrick J. Wong
Quoted 5 lines · Luis R. Rodriguez
mkfs/config.c new file +5 -0
+static struct confopts *
+get_confopts(
+	const char	*section)
+{
+	unsigned int	i;

mkfs/config.c line 250

Quoted 10 lines
mkfs/config.c new file +10 -0
+	struct confopts	*opts;
+
+	for (i=0; i < ARRAY_SIZE(confopts_tab); i++) {
+		opts = &confopts_tab[i];
+		if (!opts)
+			return NULL;
+		if (strcmp(opts->name, section) == 0) {
+			if (opts->seen) {
+				fprintf(stderr, _("Section '%s' respecified\n"),
+						section);

If I have two [data] sections, will this resuilt in:

Quoted 5 lines
# mkfs.xfs -c foo /dev/sda1
Section 'data' respecified
Invalid section on line foo:1 [data]

?

Yeah.

Quoted 7 lines
Quoted 1 line

The section isn't invalid, it's just double-specified, so...

I've fixed this, now we get:

ergon:~ # mkfs.xfs -f /dev/loop5 -c bar
Section 'metadata' respecified
Error parsing command line config file: bar : Invalid argument

Ok, thanks!

Quoted 71 lines
Quoted 8 lines

mkfs/config.c line 256

Quoted 6 lines
mkfs/config.c new file +6 -0
+				return NULL;
+			}
+			opts->seen = true;
+			return opts;
+		}
+	}

...I'd print the 'Invalid section' error message here.

Or move the respecification error message check to the switch statement handlig the section. I went with the later.

Quoted 50 lines
Quoted 50 lines
mkfs/config.c new file +50 -0
+static const char *conf_paths[] = {
+	".",
+	MKFS_XFS_CONF_DIR,
+};
+
+/*
+ * If the file is not found -1 is returned and errno set. Otherwise
+ * the file descriptor is returned.
+ */
+int
+open_cli_config(
+	char			*cli_config_file,
+	char			**fpath)
+{
+	int			fd, len;
+	char			*final_path = NULL;
+	char			*relative_path= NULL;
+	unsigned int		i;
+
+	if (strlen(cli_config_file) > 2) {
+		if (cli_config_file[0] == '.' && cli_config_file[1] == '/')
+			final_path = cli_config_file;
+		else if (cli_config_file[0] == '.' && cli_config_file[1] == '.')
+			final_path = cli_config_file;
+		else if (cli_config_file[0] == '/')
+			final_path = cli_config_file;
+		else
+			relative_path = cli_config_file;
+	} else if (strlen(cli_config_file) == 1) {
+		if (cli_config_file[0] == '.' || cli_config_file[0] == '/') {
+			errno = EINVAL;
+			return -1;
+		} else
+			relative_path = cli_config_file;
+	}
+
+	if (final_path) {
+		fd = open(final_path, O_RDONLY);
+		if (fd >= 0)
+			memcpy(*fpath, final_path, strlen(final_path));
+		return fd;
+	}
+
+	/* We finally know the path is relative but just to be sure */
+	if (!relative_path) {
+		errno = ENXIO;
+		return -1;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(conf_paths); i++) {
Quoted 8 lines

mkfs/config.c line 534

Quoted 5 lines
mkfs/config.c new file +5 -0
+		memset(*fpath, 0, PATH_MAX);
+		/*
+		 * For current directory evaluation, skip concatenating the
+		 * ./ from the file passed. We only concatenate for the other
+		 * paths we look up on.

If conf_paths[0] was "./" then you wouldn't have to special case this, I think.

Quoted 2 lines

No, I had tried it, it looks odd still, if the user specified -c foo, the output should show ./foo, not just foo.

Ah. Good point.

Quoted 8 lines
Quoted 7 lines

mkfs/config.c line 608

Quoted 5 lines
mkfs/config.c new file +5 -0
+	fp = fdopen(fd, "r");
+	if (!fp) {
+		ret = errno;
+		fprintf(stderr, _("Unable to open stream for config file: %s : %s\n"),
+				fpath, strerror(errno));

perror(fpath); ?

Quoted 6 lines

Sure.

Quoted 1 line

Looks good otherwise.

Groovy. I'll wait for others to comment otherwise I can spin up 4th iteration after the weekend.

<nod>

--D

Quoted 1 line

Luis

author
#10 lore raw

PATCH v3 4/4 debian/rules: use the new sysconfdir configuration setting

In reply to Luis R. Rodriguez

Configuration file support requires setting --sysconfdir=/etc/ if your distribution expects mkfs.xfs.d directory to be in /etc.

Signed-off-by Luis R. Rodriguez <[email protected]>
---
 debian/rules | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
debian/rules +2 -2
index cb4fa22c1584..599d7998052e 100755
@@ -20,9 +20,9 @@ stdenv = @GZIP=-q; export GZIP;
 
 options = export DEBUG=-DNDEBUG DISTRIBUTION=debian \
 	  INSTALL_USER=root INSTALL_GROUP=root \
-	  LOCAL_CONFIGURE_OPTIONS="--enable-readline=yes --enable-blkid=yes --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto" ;
+	  LOCAL_CONFIGURE_OPTIONS="--enable-readline=yes --enable-blkid=yes --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto --sysconfdir=/etc/" ;
 diopts  = $(options) \
-	  export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto" ;
+	  export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto --sysconfdir=/etc/" ;
 checkdir = test -f debian/rules
 
 build: build-arch build-indep
Signature · 1 line
2.16.3
Reviewed-by on v3 4/4
#11 lore raw

Re: [PATCH v3 4/4] debian/rules: use the new sysconfdir configuration setting

Quoted 4 lines · Luis R. Rodriguez

Configuration file support requires setting --sysconfdir=/etc/ if your distribution expects mkfs.xfs.d directory to be in /etc.

Signed-off-by Luis R. Rodriguez <[email protected]>

Looks good, thanks for putting this series together!

Reviewed-by Darrick J. Wong <[email protected]>

--D

Quoted 27 lines
---
 debian/rules | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
debian/rules +2 -2
index cb4fa22c1584..599d7998052e 100755
@@ -20,9 +20,9 @@ stdenv = @GZIP=-q; export GZIP;
 
 options = export DEBUG=-DNDEBUG DISTRIBUTION=debian \
 	  INSTALL_USER=root INSTALL_GROUP=root \
-	  LOCAL_CONFIGURE_OPTIONS="--enable-readline=yes --enable-blkid=yes --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto" ;
+	  LOCAL_CONFIGURE_OPTIONS="--enable-readline=yes --enable-blkid=yes --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto --sysconfdir=/etc/" ;
 diopts  = $(options) \
-	  export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto" ;
+	  export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto --sysconfdir=/etc/" ;
 checkdir = test -f debian/rules
 
 build: build-arch build-indep
Signature · 6 lines
2.16.3

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html