[PATCH v3 0/3] xfs_quota: allow operation on ext4 for project quotas

[PATCH v3 0/3] xfs_quota: allow operation on ext4 for project quotas

[Bill O'Donnell]

Hello -

I'm submitting a version 3 to better document the introduction of
the "-f" (foreign filesystem) flag, along with some minor refactoring
of the init_args_command and init_check_command in quota/init.c, to
proper functionality and better readability.

Also, for this series, patch 2 was superfluous as it was already
committed (commit 83f4b5a), so remove it from the series.

This is a resubmission of Dave Chinner's original 2-patch series
to enable using xfs_quota for project quotas on foreign filesystems
(e.g. ext4).

Original series: http://oss.sgi.com/archives/xfs/2016-02/msg00107.html

Updated series:
Patch 1: initial capabilities to enable xfs_quota use on foreign filesystems.
Patch 2: REMOVED (superfluous)
Patch 3: additional changes to accomodate xfs_quota use on foreign filesystems.

Questions and comments are welcome.

Thanks-
Bill

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

[PATCH v3 1/2] xfs_quota: add capabilities for use on ext4

[Bill O'Donnell]

This patch allows xfs_quota to be used on ext4 for project
quota testing in xfstests.

This patch was originally submitted by Dave Chinner
(http://oss.sgi.com/archives/xfs/2016-02/msg00131.html)

Notes:
As a part of its support for foreign filesystems xfs_quota
is modified with a "-f" command line flag to enable select
commands on those filesystems. With this addition, some
of the logic, notably in quota/init.c requires adjustment.

The original init_args_command() code in init.c searched
for the first mount point entry in the filesystem table
(i.e. a FS_MOUNT_POINT entry) and it did so by skipping
over FS_PROJECT_PATH entries. Once it found an entry that
was not a project quota path entry, it stopped, since it now
had an XFS mount point to work from.

In the original submitted patch series:
(http://oss.sgi.com/archives/xfs/2016-02/msg00131.html),
a subtle change in init_args_command() introduced an error,
preventing correct break from its do-while loop, ultimately
resulting in xfs quota test failure. This new patch includes
modifications to that logic to effect proper break from the
loop for xfs and foreign filesystems.

With the introduction of the foreign file system flag, make
usage and man-page changes accordingly. If thrown, the -f
flag will allow select user and administrative commands to
be run on mounted foreign filesystems (e.g. ext4).

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 include/command.h    |  3 ++-
 include/path.h       |  1 +
 io/init.h            |  2 +-
 libxcmd/paths.c      |  7 +++----
 man/man8/xfs_quota.8 |  7 +++++++
 quota/free.c         |  2 ++
 quota/init.c         | 37 ++++++++++++++++++++++++++++++++++---
 quota/init.h         |  1 +
 quota/path.c         |  5 +++--
 quota/project.c      |  1 +
 quota/quot.c         |  1 +
 quota/quota.c        |  2 ++
 quota/report.c       | 11 +++++++++--
 quota/state.c        |  4 +++-
 quota/util.c         |  1 +
 15 files changed, 71 insertions(+), 14 deletions(-)

diff --git a/include/command.h b/include/command.h
index 7b9fc28..81d5a4d 100644
--- a/include/command.h
+++ b/include/command.h
@@ -20,7 +20,8 @@
 
 #include <sys/time.h>
 
-#define CMD_FLAG_GLOBAL	((int)0x80000000)	/* don't iterate "args" */
+#define CMD_FLAG_GLOBAL		(1<<31)	/* don't iterate "args" */
+#define CMD_FLAG_FOREIGN_OK	(1<<30)	/* command not restricted to XFS */
 
 typedef int (*cfunc_t)(int argc, char **argv);
 typedef void (*helpfunc_t)(void);
diff --git a/include/path.h b/include/path.h
index 46a887e..39c1a95 100644
--- a/include/path.h
+++ b/include/path.h
@@ -29,6 +29,7 @@
 
 #define FS_MOUNT_POINT	(1<<0)
 #define FS_PROJECT_PATH	(1<<1)
+#define FS_FOREIGN	(1<<2)
 
 typedef struct fs_path {
 	char		*fs_name;	/* Data device for filesystem 	*/
diff --git a/io/init.h b/io/init.h
index d773b1b..bb25242 100644
--- a/io/init.h
+++ b/io/init.h
@@ -18,7 +18,7 @@
 
 #define CMD_NOFILE_OK	(1<<0)	/* command doesn't need an open file	*/
 #define CMD_NOMAP_OK	(1<<1)	/* command doesn't need a mapped region	*/
-#define CMD_FOREIGN_OK	(1<<2)	/* command not restricted to XFS files	*/
+#define CMD_FOREIGN_OK	CMD_FLAG_FOREIGN_OK
 
 extern char	*progname;
 extern int	exitcode;
diff --git a/libxcmd/paths.c b/libxcmd/paths.c
index 71af25f..7c8c673 100644
--- a/libxcmd/paths.c
+++ b/libxcmd/paths.c
@@ -113,6 +113,9 @@ fs_table_insert(
 			goto out_nodev;
 	}
 
+	if (!platform_test_xfs_path(dir))
+		flags |= FS_FOREIGN;
+
 	/*
 	 * Make copies of the directory and data device path.
 	 * The log device and real-time device, if non-null,
@@ -301,8 +304,6 @@ fs_table_initialise_mounts(
 			return errno;
 
 	while ((mnt = getmntent(mtp)) != NULL) {
-		if (strcmp(mnt->mnt_type, "xfs") != 0)
-			continue;
 		if (!realpath(mnt->mnt_dir, rmnt_dir))
 			continue;
 		if (!realpath(mnt->mnt_fsname, rmnt_fsname))
@@ -360,8 +361,6 @@ fs_table_initialise_mounts(
 			return errno;
 
 	for (i = 0; i < count; i++) {
-		if (strcmp(stats[i].f_fstypename, "xfs") != 0)
-			continue;
 		if (!realpath(stats[i].f_mntfromname, rmntfromname))
 			continue;
 		if (!realpath(stats[i].f_mntonname, rmntonname))
diff --git a/man/man8/xfs_quota.8 b/man/man8/xfs_quota.8
index f66e421..03add20 100644
--- a/man/man8/xfs_quota.8
+++ b/man/man8/xfs_quota.8
@@ -6,6 +6,8 @@ xfs_quota \- manage use of quota on XFS filesystems
 [
 .B \-x
 ] [
+.B \-f
+] [
 .B \-p
 .I prog
 ] [
@@ -51,6 +53,11 @@ All of the administrative commands (see the ADMINISTRATOR COMMANDS
 section below) which allow modifications to the quota system are
 available only in expert mode.
 .TP
+.B \-f
+Enable foreign filesystem mode.
+A limited number of user and administrative commands are available for
+use on some foreign (non-XFS) filesystems.
+.TP
 .BI \-d " project"
 Project names or numeric identifiers may be specified with this option,
 which restricts the output of the individual
diff --git a/quota/free.c b/quota/free.c
index e9e0319..b9be954 100644
--- a/quota/free.c
+++ b/quota/free.c
@@ -16,6 +16,7 @@
  * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
+#include <stdbool.h>
 #include "command.h"
 #include "init.h"
 #include "quota.h"
@@ -371,6 +372,7 @@ free_init(void)
 	free_cmd.args = _("[-bir] [-hn] [-f file]");
 	free_cmd.oneline = _("show free and used counts for blocks and inodes");
 	free_cmd.help = free_help;
+	free_cmd.flags = CMD_FLAG_FOREIGN_OK;
 
 	add_command(&free_cmd);
 }
diff --git a/quota/init.c b/quota/init.c
index 52f7941..137cd68 100644
--- a/quota/init.c
+++ b/quota/init.c
@@ -24,6 +24,7 @@
 char	*progname;
 int	exitcode;
 int	expert;
+bool	foreign_allowed = false;
 
 static char **projopts;	/* table of project names (cmdline) */
 static int nprojopts;	/* number of entries in name table. */
@@ -45,7 +46,7 @@ static void
 usage(void)
 {
 	fprintf(stderr,
-		_("Usage: %s [-V] [-x] [-p prog] [-c cmd]... [-d project]... [path]\n"),
+		_("Usage: %s [-V] [-x] [-f] [-p prog] [-c cmd]... [-d project]... [path]\n"),
 		progname);
 	exit(1);
 }
@@ -83,15 +84,42 @@ init_args_command(
 
 	do {
 		fs_path = &fs_table[index++];
-	} while ((fs_path->fs_flags & FS_PROJECT_PATH) && index < fs_count);
+		/* skip project quota entries */
+		if ((fs_path->fs_flags & FS_PROJECT_PATH))
+			continue;
+
+		/* only consider foreign filesystems if told so */
+		if (!foreign_allowed && (fs_path->fs_flags & FS_FOREIGN))
+			continue;
+
+		/* We can use this one */
+		break;
+	} while (index < fs_count);
 
 	if (fs_path->fs_flags & FS_PROJECT_PATH)
 		return 0;
+	if (!foreign_allowed && (fs_path->fs_flags & FS_FOREIGN))
+		return 0;
 	if (index > fs_count)
 		return 0;
 	return index;
 }
 
+static int
+init_check_command(
+	const cmdinfo_t	*ct)
+{
+	if (fs_path &&
+	    !(ct->flags & CMD_FLAG_FOREIGN_OK) &&
+	     (fs_path->fs_flags & FS_FOREIGN)) {
+		fprintf(stderr,
+	_("foreign mount active, %s command is for XFS filesystems only\n"),
+			ct->name);
+		return 0;
+	}
+	return 1;
+}
+
 static void
 init(
 	int		argc,
@@ -104,7 +132,7 @@ init(
 	bindtextdomain(PACKAGE, LOCALEDIR);
 	textdomain(PACKAGE);
 
-	while ((c = getopt(argc, argv, "c:d:D:P:p:t:xV")) != EOF) {
+	while ((c = getopt(argc, argv, "c:d:D:fP:p:t:xV")) != EOF) {
 		switch (c) {
 		case 'c':	/* commands */
 			add_user_command(optarg);
@@ -112,6 +140,8 @@ init(
 		case 'd':
 			add_project_opt(optarg);
 			break;
+		case 'f':
+			foreign_allowed = true;
 		case 't':
 			mtab_file = optarg;
 			break;
@@ -140,6 +170,7 @@ init(
 
 	init_commands();
 	add_args_command(init_args_command);
+	add_check_command(init_check_command);
 
 	/*
 	 * Ensure that global commands don't end up with an invalid path pointer
diff --git a/quota/init.h b/quota/init.h
index 71706cb..6879855 100644
--- a/quota/init.h
+++ b/quota/init.h
@@ -19,6 +19,7 @@
 extern char	*progname;
 extern int	exitcode;
 extern int	expert;
+extern bool	foreign_allowed;
 
 extern void	edit_init(void);
 extern void	free_init(void);
diff --git a/quota/path.c b/quota/path.c
index bdb8c98..a623d25 100644
--- a/quota/path.c
+++ b/quota/path.c
@@ -42,6 +42,7 @@ printpath(
 	if (number) {
 		printf(_("%c%03d%c "), braces? '[':' ', index, braces? ']':' ');
 	}
+	printf("%s ", (path->fs_flags & FS_FOREIGN) ? "(F)" : "   ");
 	printf(_("%-19s %s"), path->fs_dir, path->fs_name);
 	if (path->fs_flags & FS_PROJECT_PATH) {
 		prj = getprprid(path->fs_prid);
@@ -127,7 +128,7 @@ path_init(void)
 	path_cmd.cfunc = path_f;
 	path_cmd.argmin = 0;
 	path_cmd.argmax = 1;
-	path_cmd.flags = CMD_FLAG_GLOBAL;
+	path_cmd.flags = CMD_FLAG_GLOBAL | CMD_FLAG_FOREIGN_OK;
 	path_cmd.oneline = _("set current path, or show the list of paths");
 
 	print_cmd.name = "print";
@@ -135,7 +136,7 @@ path_init(void)
 	print_cmd.cfunc = print_f;
 	print_cmd.argmin = 0;
 	print_cmd.argmax = 0;
-	print_cmd.flags = CMD_FLAG_GLOBAL;
+	print_cmd.flags = CMD_FLAG_GLOBAL | CMD_FLAG_FOREIGN_OK;
 	print_cmd.oneline = _("list known mount points and projects");
 
 	if (expert)
diff --git a/quota/project.c b/quota/project.c
index fb8b9e1..e4e7a01 100644
--- a/quota/project.c
+++ b/quota/project.c
@@ -355,6 +355,7 @@ project_init(void)
 	project_cmd.argmax = -1;
 	project_cmd.oneline = _("check, setup or clear project quota trees");
 	project_cmd.help = project_help;
+	project_cmd.flags = CMD_FLAG_FOREIGN_OK;
 
 	if (expert)
 		add_command(&project_cmd);
diff --git a/quota/quot.c b/quota/quot.c
index 2e583e5..ccc154f 100644
--- a/quota/quot.c
+++ b/quota/quot.c
@@ -16,6 +16,7 @@
  * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
+#include <stdbool.h>
 #include "command.h"
 #include <ctype.h>
 #include <pwd.h>
diff --git a/quota/quota.c b/quota/quota.c
index e0da7c0..d09e239 100644
--- a/quota/quota.c
+++ b/quota/quota.c
@@ -16,6 +16,7 @@
  * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
+#include <stdbool.h>
 #include "command.h"
 #include <ctype.h>
 #include <pwd.h>
@@ -469,6 +470,7 @@ quota_init(void)
 	quota_cmd.args = _("[-bir] [-g|-p|-u] [-hnNv] [-f file] [id|name]...");
 	quota_cmd.oneline = _("show usage and limits");
 	quota_cmd.help = quota_help;
+	quota_cmd.flags = CMD_FLAG_FOREIGN_OK;
 
 	add_command(&quota_cmd);
 }
diff --git a/quota/report.c b/quota/report.c
index 70220b4..604f50d 100644
--- a/quota/report.c
+++ b/quota/report.c
@@ -15,7 +15,7 @@
  * along with this program; if not, write the Free Software Foundation,
  * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
-
+#include <stdbool.h>
 #include "command.h"
 #include <sys/types.h>
 #include <pwd.h>
@@ -618,6 +618,8 @@ report_any_type(
 	if (type & XFS_USER_QUOTA) {
 		fs_cursor_initialise(dir, FS_MOUNT_POINT, &cursor);
 		while ((mount = fs_cursor_next_entry(&cursor))) {
+			if (!foreign_allowed && (mount->fs_flags & FS_FOREIGN))
+				continue;
 			if (xfsquotactl(XFS_QSYNC, mount->fs_name,
 						XFS_USER_QUOTA, 0, NULL) < 0
 					&& errno != ENOENT && errno != ENOSYS)
@@ -629,6 +631,8 @@ report_any_type(
 	if (type & XFS_GROUP_QUOTA) {
 		fs_cursor_initialise(dir, FS_MOUNT_POINT, &cursor);
 		while ((mount = fs_cursor_next_entry(&cursor))) {
+			if (!foreign_allowed && (mount->fs_flags & FS_FOREIGN))
+				continue;
 			if (xfsquotactl(XFS_QSYNC, mount->fs_name,
 						XFS_GROUP_QUOTA, 0, NULL) < 0
 					&& errno != ENOENT && errno != ENOSYS)
@@ -640,6 +644,8 @@ report_any_type(
 	if (type & XFS_PROJ_QUOTA) {
 		fs_cursor_initialise(dir, FS_MOUNT_POINT, &cursor);
 		while ((mount = fs_cursor_next_entry(&cursor))) {
+			if (!foreign_allowed && (mount->fs_flags & FS_FOREIGN))
+				continue;
 			if (xfsquotactl(XFS_QSYNC, mount->fs_name,
 						XFS_PROJ_QUOTA, 0, NULL) < 0
 					&& errno != ENOENT && errno != ENOSYS)
@@ -754,16 +760,17 @@ report_init(void)
 	dump_cmd.args = _("[-g|-p|-u] [-f file]");
 	dump_cmd.oneline = _("dump quota information for backup utilities");
 	dump_cmd.help = dump_help;
+	dump_cmd.flags = CMD_FLAG_FOREIGN_OK;
 
 	report_cmd.name = "report";
 	report_cmd.altname = "repquota";
 	report_cmd.cfunc = report_f;
 	report_cmd.argmin = 0;
 	report_cmd.argmax = -1;
-	report_cmd.flags = CMD_FLAG_GLOBAL;
 	report_cmd.args = _("[-bir] [-gpu] [-ahnt] [-f file]");
 	report_cmd.oneline = _("report filesystem quota information");
 	report_cmd.help = report_help;
+	report_cmd.flags = CMD_FLAG_GLOBAL | CMD_FLAG_FOREIGN_OK;
 
 	if (expert) {
 		add_command(&dump_cmd);
diff --git a/quota/state.c b/quota/state.c
index 8186762..d134580 100644
--- a/quota/state.c
+++ b/quota/state.c
@@ -15,7 +15,7 @@
  * along with this program; if not, write the Free Software Foundation,
  * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
-
+#include <stdbool.h>
 #include "command.h"
 #include "init.h"
 #include "quota.h"
@@ -527,6 +527,7 @@ state_init(void)
 	off_cmd.args = _("[-gpu] [-v]");
 	off_cmd.oneline = _("permanently switch quota off for a path");
 	off_cmd.help = off_help;
+	off_cmd.flags = CMD_FLAG_FOREIGN_OK;
 
 	state_cmd.name = "state";
 	state_cmd.cfunc = state_f;
@@ -535,6 +536,7 @@ state_init(void)
 	state_cmd.args = _("[-gpu] [-a] [-v] [-f file]");
 	state_cmd.oneline = _("get overall quota state information");
 	state_cmd.help = state_help;
+	state_cmd.flags = CMD_FLAG_FOREIGN_OK;
 
 	enable_cmd.name = "enable";
 	enable_cmd.cfunc = enable_f;
diff --git a/quota/util.c b/quota/util.c
index e3c5398..cafd45f 100644
--- a/quota/util.c
+++ b/quota/util.c
@@ -17,6 +17,7 @@
  */
 
 #include <sys/types.h>
+#include <stdbool.h>
 #include <pwd.h>
 #include <grp.h>
 #include <utmp.h>
-- 
2.7.4

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

[PATCH v3 2/2] xfs_quota: additional changes to allow use on ext4

[Bill O'Donnell]

Further changes to allow xfs_quota to be used on foreign filesystem(s)
(e.g. ext4) for project quota testing in xfstests.

Add CMD_SKIP_CHECK to enable "generic" xfs_quota commands (help and
quit) when xfs_quota is run on foreign filesystems.
Use CMD_FLAG_FOREIGN_OK on commands suitable for foreign filesystems.
Refactor init_check_command in quota/init.c for clarity.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 libxcmd/help.c |  4 +++-
 libxcmd/quit.c |  4 +++-
 quota/init.c   | 29 ++++++++++++++++++++---------
 quota/init.h   |  2 ++
 4 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/libxcmd/help.c b/libxcmd/help.c
index fad0ab9..d2e72a0 100644
--- a/libxcmd/help.c
+++ b/libxcmd/help.c
@@ -18,6 +18,7 @@
 
 #include "platform_defs.h"
 #include "command.h"
+#include "../quota/init.h"
 
 static cmdinfo_t help_cmd;
 static void help_onecmd(const char *cmd, const cmdinfo_t *ct);
@@ -88,7 +89,8 @@ help_init(void)
 	help_cmd.cfunc = help_f;
 	help_cmd.argmin = 0;
 	help_cmd.argmax = 1;
-	help_cmd.flags = CMD_FLAG_GLOBAL;
+	help_cmd.flags = CMD_FLAG_GLOBAL | CMD_FLAG_FOREIGN_OK |
+		CMD_SKIP_CHECK;
 	help_cmd.args = _("[command]");
 	help_cmd.oneline = _("help for one or all commands");
 
diff --git a/libxcmd/quit.c b/libxcmd/quit.c
index 0183b8f..9dbd29a 100644
--- a/libxcmd/quit.c
+++ b/libxcmd/quit.c
@@ -18,6 +18,7 @@
 
 #include "platform_defs.h"
 #include "command.h"
+#include "../quota/init.h"
 
 static cmdinfo_t quit_cmd;
 
@@ -38,7 +39,8 @@ quit_init(void)
 	quit_cmd.cfunc = quit_f;
 	quit_cmd.argmin = -1;
 	quit_cmd.argmax = -1;
-	quit_cmd.flags = CMD_FLAG_GLOBAL;
+	quit_cmd.flags = CMD_FLAG_GLOBAL | CMD_FLAG_FOREIGN_OK |
+		CMD_SKIP_CHECK;
 	quit_cmd.oneline = _("exit the program");
 
 	add_command(&quit_cmd);
diff --git a/quota/init.c b/quota/init.c
index 137cd68..b487dd5 100644
--- a/quota/init.c
+++ b/quota/init.c
@@ -109,15 +109,26 @@ static int
 init_check_command(
 	const cmdinfo_t	*ct)
 {
-	if (fs_path &&
-	    !(ct->flags & CMD_FLAG_FOREIGN_OK) &&
-	     (fs_path->fs_flags & FS_FOREIGN)) {
-		fprintf(stderr,
-	_("foreign mount active, %s command is for XFS filesystems only\n"),
-			ct->name);
-		return 0;
-	}
-	return 1;
+	if (!fs_path)
+		return 1;
+
+	/* Always run commands that we are told to skip here */
+	if (ct->flags & CMD_SKIP_CHECK)
+		return 1;
+
+	/* if it's an XFS filesystem, always run the command */
+	if (!(fs_path->fs_flags & FS_FOREIGN))
+		return 1;
+
+	/* If the user specified foreign filesysetms are ok, run it */
+	if (foreign_allowed &&
+	    (ct->flags & CMD_FLAG_FOREIGN_OK))
+		return 1;
+
+	/* foreign filesystem and it's no a valid command! */
+	fprintf(stderr, _("%s command is for XFS filesystems only\n"),
+		ct->name);
+	return 0;
 }
 
 static void
diff --git a/quota/init.h b/quota/init.h
index 6879855..aa1cc51 100644
--- a/quota/init.h
+++ b/quota/init.h
@@ -31,3 +31,5 @@ extern void	report_init(void);
 extern void	state_init(void);
 
 extern void init_cvtnum(unsigned int *, unsigned int *);
+
+#define CMD_SKIP_CHECK	(1<<0) /* command is always run */
-- 
2.7.4

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

Re: [PATCH v3 2/2] xfs_quota: additional changes to allow use on ext4

[Eric Sandeen]

On 8/23/16 11:40 AM, Bill O'Donnell wrote:
> Further changes to allow xfs_quota to be used on foreign filesystem(s)
> (e.g. ext4) for project quota testing in xfstests.
> 
> Add CMD_SKIP_CHECK to enable "generic" xfs_quota commands (help and
> quit) when xfs_quota is run on foreign filesystems.
> Use CMD_FLAG_FOREIGN_OK on commands suitable for foreign filesystems.
> Refactor init_check_command in quota/init.c for clarity.
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> ---
>  libxcmd/help.c |  4 +++-
>  libxcmd/quit.c |  4 +++-
>  quota/init.c   | 29 ++++++++++++++++++++---------
>  quota/init.h   |  2 ++
>  4 files changed, 28 insertions(+), 11 deletions(-)
> 
> diff --git a/libxcmd/help.c b/libxcmd/help.c
> index fad0ab9..d2e72a0 100644
> --- a/libxcmd/help.c
> +++ b/libxcmd/help.c
> @@ -18,6 +18,7 @@
>  
>  #include "platform_defs.h"
>  #include "command.h"
> +#include "../quota/init.h"
>  
>  static cmdinfo_t help_cmd;
>  static void help_onecmd(const char *cmd, const cmdinfo_t *ct);
> @@ -88,7 +89,8 @@ help_init(void)
>  	help_cmd.cfunc = help_f;
>  	help_cmd.argmin = 0;
>  	help_cmd.argmax = 1;
> -	help_cmd.flags = CMD_FLAG_GLOBAL;
> +	help_cmd.flags = CMD_FLAG_GLOBAL | CMD_FLAG_FOREIGN_OK |
> +		CMD_SKIP_CHECK;

Nitpick, but no need to set both FOREIGN_OK and SKIP_CHECK, right?
If the first is set, the 2nd is never tested (below), I think?

>  	help_cmd.args = _("[command]");
>  	help_cmd.oneline = _("help for one or all commands");
>  
> diff --git a/libxcmd/quit.c b/libxcmd/quit.c
> index 0183b8f..9dbd29a 100644
> --- a/libxcmd/quit.c
> +++ b/libxcmd/quit.c
> @@ -18,6 +18,7 @@
>  
>  #include "platform_defs.h"
>  #include "command.h"
> +#include "../quota/init.h"
>  
>  static cmdinfo_t quit_cmd;
>  
> @@ -38,7 +39,8 @@ quit_init(void)
>  	quit_cmd.cfunc = quit_f;
>  	quit_cmd.argmin = -1;
>  	quit_cmd.argmax = -1;
> -	quit_cmd.flags = CMD_FLAG_GLOBAL;
> +	quit_cmd.flags = CMD_FLAG_GLOBAL | CMD_FLAG_FOREIGN_OK |
> +		CMD_SKIP_CHECK;
>  	quit_cmd.oneline = _("exit the program");
>  
>  	add_command(&quit_cmd);
> diff --git a/quota/init.c b/quota/init.c
> index 137cd68..b487dd5 100644
> --- a/quota/init.c
> +++ b/quota/init.c
> @@ -109,15 +109,26 @@ static int
>  init_check_command(
>  	const cmdinfo_t	*ct)
>  {
> -	if (fs_path &&
> -	    !(ct->flags & CMD_FLAG_FOREIGN_OK) &&
> -	     (fs_path->fs_flags & FS_FOREIGN)) {
> -		fprintf(stderr,
> -	_("foreign mount active, %s command is for XFS filesystems only\n"),
> -			ct->name);
> -		return 0;
> -	}
> -	return 1;
> +	if (!fs_path)
> +		return 1;
> +
> +	/* Always run commands that we are told to skip here */
> +	if (ct->flags & CMD_SKIP_CHECK)
> +		return 1;
> +
> +	/* if it's an XFS filesystem, always run the command */
> +	if (!(fs_path->fs_flags & FS_FOREIGN))
> +		return 1;
> +
> +	/* If the user specified foreign filesysetms are ok, run it */
> +	if (foreign_allowed &&
> +	    (ct->flags & CMD_FLAG_FOREIGN_OK))
> +		return 1;
> +
> +	/* foreign filesystem and it's no a valid command! */

s/no/not/ while you're at it :)

> +	fprintf(stderr, _("%s command is for XFS filesystems only\n"),
> +		ct->name);
> +	return 0;
>  }
>  
>  static void
> diff --git a/quota/init.h b/quota/init.h
> index 6879855..aa1cc51 100644
> --- a/quota/init.h
> +++ b/quota/init.h
> @@ -31,3 +31,5 @@ extern void	report_init(void);
>  extern void	state_init(void);
>  
>  extern void init_cvtnum(unsigned int *, unsigned int *);
> +
> +#define CMD_SKIP_CHECK	(1<<0) /* command is always run */

how about "always available" - it's not run unless it's invoked.  ;)

-Eric

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

Re: [PATCH v3 2/2] xfs_quota: additional changes to allow use on ext4

[Bill O'Donnell]

On Tue, Aug 23, 2016 at 11:54:48AM -0500, Eric Sandeen wrote:
> 
> 
> On 8/23/16 11:40 AM, Bill O'Donnell wrote:
> > Further changes to allow xfs_quota to be used on foreign filesystem(s)
> > (e.g. ext4) for project quota testing in xfstests.
> > 
> > Add CMD_SKIP_CHECK to enable "generic" xfs_quota commands (help and
> > quit) when xfs_quota is run on foreign filesystems.
> > Use CMD_FLAG_FOREIGN_OK on commands suitable for foreign filesystems.
> > Refactor init_check_command in quota/init.c for clarity.
> > 
> > Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> > ---
> >  libxcmd/help.c |  4 +++-
> >  libxcmd/quit.c |  4 +++-
> >  quota/init.c   | 29 ++++++++++++++++++++---------
> >  quota/init.h   |  2 ++
> >  4 files changed, 28 insertions(+), 11 deletions(-)
> > 
> > diff --git a/libxcmd/help.c b/libxcmd/help.c
> > index fad0ab9..d2e72a0 100644
> > --- a/libxcmd/help.c
> > +++ b/libxcmd/help.c
> > @@ -18,6 +18,7 @@
> >  
> >  #include "platform_defs.h"
> >  #include "command.h"
> > +#include "../quota/init.h"
> >  
> >  static cmdinfo_t help_cmd;
> >  static void help_onecmd(const char *cmd, const cmdinfo_t *ct);
> > @@ -88,7 +89,8 @@ help_init(void)
> >  	help_cmd.cfunc = help_f;
> >  	help_cmd.argmin = 0;
> >  	help_cmd.argmax = 1;
> > -	help_cmd.flags = CMD_FLAG_GLOBAL;
> > +	help_cmd.flags = CMD_FLAG_GLOBAL | CMD_FLAG_FOREIGN_OK |
> > +		CMD_SKIP_CHECK;
> 
> Nitpick, but no need to set both FOREIGN_OK and SKIP_CHECK, right?
> If the first is set, the 2nd is never tested (below), I think?

Yes. Good catch.

> 
> >  	help_cmd.args = _("[command]");
> >  	help_cmd.oneline = _("help for one or all commands");
> >  
> > diff --git a/libxcmd/quit.c b/libxcmd/quit.c
> > index 0183b8f..9dbd29a 100644
> > --- a/libxcmd/quit.c
> > +++ b/libxcmd/quit.c
> > @@ -18,6 +18,7 @@
> >  
> >  #include "platform_defs.h"
> >  #include "command.h"
> > +#include "../quota/init.h"
> >  
> >  static cmdinfo_t quit_cmd;
> >  
> > @@ -38,7 +39,8 @@ quit_init(void)
> >  	quit_cmd.cfunc = quit_f;
> >  	quit_cmd.argmin = -1;
> >  	quit_cmd.argmax = -1;
> > -	quit_cmd.flags = CMD_FLAG_GLOBAL;
> > +	quit_cmd.flags = CMD_FLAG_GLOBAL | CMD_FLAG_FOREIGN_OK |
> > +		CMD_SKIP_CHECK;
> >  	quit_cmd.oneline = _("exit the program");
> >  
> >  	add_command(&quit_cmd);
> > diff --git a/quota/init.c b/quota/init.c
> > index 137cd68..b487dd5 100644
> > --- a/quota/init.c
> > +++ b/quota/init.c
> > @@ -109,15 +109,26 @@ static int
> >  init_check_command(
> >  	const cmdinfo_t	*ct)
> >  {
> > -	if (fs_path &&
> > -	    !(ct->flags & CMD_FLAG_FOREIGN_OK) &&
> > -	     (fs_path->fs_flags & FS_FOREIGN)) {
> > -		fprintf(stderr,
> > -	_("foreign mount active, %s command is for XFS filesystems only\n"),
> > -			ct->name);
> > -		return 0;
> > -	}
> > -	return 1;
> > +	if (!fs_path)
> > +		return 1;
> > +
> > +	/* Always run commands that we are told to skip here */
> > +	if (ct->flags & CMD_SKIP_CHECK)
> > +		return 1;
> > +
> > +	/* if it's an XFS filesystem, always run the command */
> > +	if (!(fs_path->fs_flags & FS_FOREIGN))
> > +		return 1;
> > +
> > +	/* If the user specified foreign filesysetms are ok, run it */
> > +	if (foreign_allowed &&
> > +	    (ct->flags & CMD_FLAG_FOREIGN_OK))
> > +		return 1;
> > +
> > +	/* foreign filesystem and it's no a valid command! */
> 
> s/no/not/ while you're at it :)
> 
Yep.

> > +	fprintf(stderr, _("%s command is for XFS filesystems only\n"),
> > +		ct->name);
> > +	return 0;
> >  }
> >  
> >  static void
> > diff --git a/quota/init.h b/quota/init.h
> > index 6879855..aa1cc51 100644
> > --- a/quota/init.h
> > +++ b/quota/init.h
> > @@ -31,3 +31,5 @@ extern void	report_init(void);
> >  extern void	state_init(void);
> >  
> >  extern void init_cvtnum(unsigned int *, unsigned int *);
> > +
> > +#define CMD_SKIP_CHECK	(1<<0) /* command is always run */
> 
> how about "always available" - it's not run unless it's invoked.  ;)

Agreed.

Thanks for the review-
Bill

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

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