[PATCH 0/3] xfsprogs: xfs_quota foreign fs path handling modifications

[PATCH 0/3] xfsprogs: xfs_quota foreign fs path handling modifications

[Bill O'Donnell]

Hello-

This is a new series that supercedes:
http://www.spinics.net/lists/xfs/msg42220.html -
(xfs_quota foreign fs path handling changes).

The following commits modified xfs_quota for use on foreign
filesystems:
b20b6c2 xfs_quota: certain commands must always be available
29647c8 xfs_quota: add capabilities for use on non-XFS filesystems

Included in those commits were modifications to
fs_initialise_mounts (paths.c), resulting in a reverse
compatibility issue. In particular, xfstest xfs/261 failed,
due to foreign fs paths being picked up from the fs table,
and the xfs_quota print command complaining about not being
able to print the foreign paths.

This series fixes the foreign path print problem, via
modifications to the xfs_quota paths and print operations.

patch 1: populate the fs table with xfs entries first,
maintaining mtab order, followed by foreign fs entries.

patch 2: adjust print and path formatting of xfs and
foreign fs entries, maintaining reverse compatibility.

patch 3: add the case for foreign fs in init_check_command
to cover both states of the -f (foreign_allowed) flag.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>

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

[PATCH 1/3] xfsprogs: populate fs table with xfs entries first, foreign entries last

[Bill O'Donnell]

Commits b20b6c2 and 29647c8 modified xfs_quota for use on
non-XFS filesystems. Modifications to fs_initialise_mounts
(paths.c) resulted in an xfstest fail (xfs/261), due to foreign
fs paths being picked up first from the fs table. The xfs_quota
print command then complained about not being able to print the
foreign paths, instead of previous behavior (quiet).

This patch restores correct behavior, sorting the table so that
xfs entries are first, followed by foreign fs entries. The patch
maintains the order of xfs entries and foreign entries in the
same order as mtab entries.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 libxcmd/paths.c | 12 +++++++++++-
 quota/path.c    | 21 ++++++++++++++++-----
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/libxcmd/paths.c b/libxcmd/paths.c
index 4158688..6363d40 100644
--- a/libxcmd/paths.c
+++ b/libxcmd/paths.c
@@ -34,6 +34,7 @@ extern char *progname;
 int fs_count;
 struct fs_path *fs_table;
 struct fs_path *fs_path;
+int		xfs_fs_count;
 
 char *mtab_file;
 #define PROC_MOUNTS	"/proc/self/mounts"
@@ -138,7 +139,16 @@ fs_table_insert(
 		goto out_norealloc;
 	fs_table = tmp_fs_table;
 
-	fs_path = &fs_table[fs_count];
+	/* Put foreign filesystems at the end, xfs filesystems at the front */
+	if (flags & FS_FOREIGN || fs_count == 0) {
+		fs_path = &fs_table[fs_count];
+	} else {
+		/* move foreign fs entries down, insert new one just before */
+		memmove(&fs_table[xfs_fs_count], &fs_table[xfs_fs_count - 1],
+			sizeof(fs_path_t)*(fs_count - xfs_fs_count + 1));
+		fs_path = &fs_table[xfs_fs_count];
+		xfs_fs_count++;
+	}
 	fs_path->fs_dir = dir;
 	fs_path->fs_prid = prid;
 	fs_path->fs_flags = flags;
diff --git a/quota/path.c b/quota/path.c
index a623d25..aa3d33e 100644
--- a/quota/path.c
+++ b/quota/path.c
@@ -75,9 +75,15 @@ static int
 pathlist_f(void)
 {
 	int		i;
+	struct fs_path	*path;
 
-	for (i = 0; i < fs_count; i++)
-		printpath(&fs_table[i], i, 1, &fs_table[i] == fs_path);
+	for (i = 0; i < fs_count; i++) {
+		path = &fs_table[i];
+		/* Table is ordered xfs first, then foreign */
+		if (path->fs_flags & FS_FOREIGN && !foreign_allowed)
+			break;
+		printpath(path, i, 1, path == fs_path);
+	}
 	return 0;
 }
 
@@ -98,7 +104,7 @@ path_f(
 	int		argc,
 	char		**argv)
 {
-	int	i;
+	int		i;
 
 	if (fs_count == 0) {
 		printf(_("No paths are available\n"));
@@ -113,8 +119,13 @@ path_f(
 		printf(_("value %d is out of range (0-%d)\n"),
 			i, fs_count-1);
 	} else {
-		fs_path = &fs_table[i];
-		pathlist_f();
+		for (i = 0; i < fs_count; i++) {
+			fs_path = &fs_table[i];
+			/* Table is ordered xfs first, then foreign */
+			if (fs_path->fs_flags & FS_FOREIGN && !foreign_allowed)
+				break;
+			pathlist_f();
+		}
 	}
 	return 0;
 }
-- 
2.7.4

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

[PATCH 2/3] xfs_quota: print and path output formatting: maintain reverse compatibility

[Bill O'Donnell]

This patch adjusts the formatting of the xfs_quota print and
path outputs, in order to maintain reverse compatability:
when -f flag isn't used, need to keep the output same as in
previous version.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 quota/path.c | 67 +++++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 39 insertions(+), 28 deletions(-)

diff --git a/quota/path.c b/quota/path.c
index aa3d33e..ed9c044 100644
--- a/quota/path.c
+++ b/quota/path.c
@@ -36,39 +36,50 @@ printpath(
 	int		c;
 
 	if (index == 0) {
-		printf(_("%sFilesystem          Pathname\n"),
-			number ? _("      ") : "");
+		if (foreign_allowed)
+			printf(_("%s    Filesystem          Pathname\n"),
+			       number ? _("      ") : "");
+		else
+			printf(_("%sFilesystem          Pathname\n"),
+			       number ? _("      ") : "");
 	}
 	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);
-		printf(_(" (project %u"), path->fs_prid);
-		if (prj)
-			printf(_(", %s"), prj->pr_name);
-		printf(")");
-	} else if (xfsquotactl(XFS_GETQSTAT, path->fs_name, 0, 0,
-				(void *)&qstat) == 0 && qstat.qs_flags) {
-		c = 0;
-		printf(" (");
-		if (qstat.qs_flags & XFS_QUOTA_UDQ_ENFD)
-			c = printf("uquota");
-		else if (qstat.qs_flags & XFS_QUOTA_UDQ_ACCT)
-			c = printf("uqnoenforce");
-		if (qstat.qs_flags & XFS_QUOTA_GDQ_ENFD)
-			c = printf("%sgquota", c ? ", " : "");
-		else if (qstat.qs_flags & XFS_QUOTA_GDQ_ACCT)
-			c = printf("%sgqnoenforce", c ? ", " : "");
-		if (qstat.qs_flags & XFS_QUOTA_PDQ_ENFD)
-			printf("%spquota", c ? ", " : "");
-		else if (qstat.qs_flags & XFS_QUOTA_PDQ_ACCT)
-			printf("%spqnoenforce", c ? ", " : "");
-		printf(")");
+	if (!((path->fs_flags & FS_FOREIGN) && !foreign_allowed)) {
+		printf("%s", (path->fs_flags & FS_FOREIGN) ? "(F) " : "");
+		if (path->fs_flags & FS_FOREIGN)
+			printf(_("%-19s %s"), path->fs_dir, path->fs_name);
+		else if (foreign_allowed)
+			printf(_("    %-19s %s"), path->fs_dir, path->fs_name);
+		else
+			printf(_("%-19s %s"), path->fs_dir, path->fs_name);
+		if (path->fs_flags & FS_PROJECT_PATH) {
+			prj = getprprid(path->fs_prid);
+			printf(_(" (project %u"), path->fs_prid);
+			if (prj)
+				printf(_(", %s"), prj->pr_name);
+			printf(")");
+		} else if (xfsquotactl(XFS_GETQSTAT, path->fs_name, 0, 0,
+				       (void *)&qstat) == 0 && qstat.qs_flags) {
+			c = 0;
+			printf(" (");
+			if (qstat.qs_flags & XFS_QUOTA_UDQ_ENFD)
+				c = printf("uquota");
+			else if (qstat.qs_flags & XFS_QUOTA_UDQ_ACCT)
+				c = printf("uqnoenforce");
+			if (qstat.qs_flags & XFS_QUOTA_GDQ_ENFD)
+				c = printf("%sgquota", c ? ", " : "");
+			else if (qstat.qs_flags & XFS_QUOTA_GDQ_ACCT)
+				c = printf("%sgqnoenforce", c ? ", " : "");
+			if (qstat.qs_flags & XFS_QUOTA_PDQ_ENFD)
+				printf("%spquota", c ? ", " : "");
+			else if (qstat.qs_flags & XFS_QUOTA_PDQ_ACCT)
+				printf("%spqnoenforce", c ? ", " : "");
+			printf(")");
+		}
+		printf("\n");
 	}
-	printf("\n");
 }
 
 static int
-- 
2.7.4

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

[PATCH 3/3] xfs_quota: add case for foreign fs, disabled regardless of foreign_allowed

[Bill O'Donnell]

Some commands are disallowed for foreign filesystems,
regardless of whether or not the -f flag is thrown.
Add a case for this condition and improve commenting
and output messaging accordingly in init_check_command.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 quota/init.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/quota/init.c b/quota/init.c
index 2c18c8b..7d69663 100644
--- a/quota/init.c
+++ b/quota/init.c
@@ -112,21 +112,28 @@ init_check_command(
 	if (!fs_path)
 		return 1;
 
-	/* Always run commands that we are told to skip here */
+	/* Always run commands that are valid for all fs types. */
 	if (ct->flags & CMD_ALL_FSTYPES)
 		return 1;
 
-	/* if it's an XFS filesystem, always run the command */
+	/* 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 the user specified foreign filesystems are ok (-f), run cmd. */
 	if (foreign_allowed &&
 	    (ct->flags & CMD_FLAG_FOREIGN_OK))
 		return 1;
 
-	/* foreign filesystem and it's not a valid command! */
-	fprintf(stderr, _("%s command is for XFS filesystems only\n"),
+	/* If cmd not allowed on foreign fs, regardless of -f flag, skip it. */
+	if (!(ct->flags & CMD_FLAG_FOREIGN_OK)) {
+		fprintf(stderr, _("%s: command is for XFS filesystems only\n"),
+			ct->name);
+		return 0;
+	}
+
+	/* foreign fs, but cmd only allowed via -f flag. Skip it. */
+	fprintf(stderr, _("%s: foreign filesystem. Use -f to enable.\n"),
 		ct->name);
 	return 0;
 }
-- 
2.7.4

Re: [PATCH 1/3] xfsprogs: populate fs table with xfs entries first, foreign entries last

[Eric Sandeen]

On 9/14/16 10:19 AM, Bill O'Donnell wrote:
> Commits b20b6c2 and 29647c8 modified xfs_quota for use on
> non-XFS filesystems. Modifications to fs_initialise_mounts
> (paths.c) resulted in an xfstest fail (xfs/261), due to foreign
> fs paths being picked up first from the fs table. The xfs_quota
> print command then complained about not being able to print the
> foreign paths, instead of previous behavior (quiet).
> 
> This patch restores correct behavior, sorting the table so that
> xfs entries are first, followed by foreign fs entries. The patch
> maintains the order of xfs entries and foreign entries in the
> same order as mtab entries.

Then, in functions which print all paths we can simply break at
the first foreign path if the -f switch is not specified.

> Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> ---
>  libxcmd/paths.c | 12 +++++++++++-
>  quota/path.c    | 21 ++++++++++++++++-----
>  2 files changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/libxcmd/paths.c b/libxcmd/paths.c
> index 4158688..6363d40 100644
> --- a/libxcmd/paths.c
> +++ b/libxcmd/paths.c
> @@ -34,6 +34,7 @@ extern char *progname;
>  int fs_count;
>  struct fs_path *fs_table;
>  struct fs_path *fs_path;
> +int		xfs_fs_count;

Since there are other changes to be made, this might be prettier:

 int fs_count;
+int xfs_fs_count;
 struct fs_path *fs_table;
 struct fs_path *fs_path;

>  char *mtab_file;
>  #define PROC_MOUNTS	"/proc/self/mounts"
> @@ -138,7 +139,16 @@ fs_table_insert(
>  		goto out_norealloc;
>  	fs_table = tmp_fs_table;
>  
> -	fs_path = &fs_table[fs_count];
> +	/* Put foreign filesystems at the end, xfs filesystems at the front */
> +	if (flags & FS_FOREIGN || fs_count == 0) {
> +		fs_path = &fs_table[fs_count];
> +	} else {
> +		/* move foreign fs entries down, insert new one just before */
> +		memmove(&fs_table[xfs_fs_count], &fs_table[xfs_fs_count - 1],
> +			sizeof(fs_path_t)*(fs_count - xfs_fs_count + 1));
> +		fs_path = &fs_table[xfs_fs_count];
> +		xfs_fs_count++;
> +	}
>  	fs_path->fs_dir = dir;
>  	fs_path->fs_prid = prid;
>  	fs_path->fs_flags = flags;

Ok, if we run this through valgrind it squawks.

==22170== Invalid read of size 8
==22170==    at 0x4A09C1C: memmove (mc_replace_strmem.c:1026)
==22170==    by 0x40B366: fs_table_insert (paths.c:149)
==22170==    by 0x40B58E: fs_table_initialise_mounts (paths.c:337)
==22170==    by 0x40BA8E: fs_table_initialise (paths.c:516)
==22170==    by 0x401CC4: main (init.c:180)
==22170==  Address 0x4c25cb8 is 8 bytes before a block of size 192 alloc'd
==22170==    at 0x4A06BE0: realloc (vg_replace_malloc.c:662)
==22170==    by 0x40B258: fs_table_insert (paths.c:137)
==22170==    by 0x40B58E: fs_table_initialise_mounts (paths.c:337)
==22170==    by 0x40BA8E: fs_table_initialise (paths.c:516)
==22170==    by 0x401CC4: main (init.c:180)
==22170==

As I mentioned, my quick suggestion may have had off-by-ones, and it did.
I think this should fix it, but *check it* ;)

        } else {
                /* move foreign fs entries down, insert new one just before */
-               memmove(&fs_table[xfs_fs_count], &fs_table[xfs_fs_count - 1],
-                       sizeof(fs_path_t)*(fs_count - xfs_fs_count + 1));
+               memmove(&fs_table[xfs_fs_count + 1], &fs_table[xfs_fs_count],
+                       sizeof(fs_path_t)*(fs_count - xfs_fs_count));
                fs_path = &fs_table[xfs_fs_count];
                xfs_fs_count++;
        }

> diff --git a/quota/path.c b/quota/path.c
> index a623d25..aa3d33e 100644
> --- a/quota/path.c
> +++ b/quota/path.c
> @@ -75,9 +75,15 @@ static int
>  pathlist_f(void)
>  {
>  	int		i;
> +	struct fs_path	*path;
>  
> -	for (i = 0; i < fs_count; i++)
> -		printpath(&fs_table[i], i, 1, &fs_table[i] == fs_path);
> +	for (i = 0; i < fs_count; i++) {
> +		path = &fs_table[i];
> +		/* Table is ordered xfs first, then foreign */
> +		if (path->fs_flags & FS_FOREIGN && !foreign_allowed)
> +			break;
> +		printpath(path, i, 1, path == fs_path);
> +	}
>  	return 0;
>  }
>  
> @@ -98,7 +104,7 @@ path_f(
>  	int		argc,
>  	char		**argv)
>  {
> -	int	i;
> +	int		i;
>  
>  	if (fs_count == 0) {
>  		printf(_("No paths are available\n"));
> @@ -113,8 +119,13 @@ path_f(

Oops, I mentioned this to you as needed, but it's wrong, sorry.

Just drop this hunk, this is supposed to select path i, not iterate
over anything.  Sorry for mentioning it.

>  		printf(_("value %d is out of range (0-%d)\n"),
>  			i, fs_count-1);
>  	} else {
> -		fs_path = &fs_table[i];
> -		pathlist_f();
> +		for (i = 0; i < fs_count; i++) {
> +			fs_path = &fs_table[i];
> +			/* Table is ordered xfs first, then foreign */
> +			if (fs_path->fs_flags & FS_FOREIGN && !foreign_allowed)
> +				break;
> +			pathlist_f();
> +		}
>  	}
>  	return 0;
>  }
> 

Another oddity:

# quota/xfs_quota -x
xfs_quota> path
      Filesystem          Pathname
[000]     /home               /dev/mapper/vg_bp05-lv_home
 001      /mnt/test2          /dev/sdc1

<3 foreign paths are loaded into the table but ignored, no "-f">

xfs_quota> path 3
      Filesystem          Pathname
 000      /home               /dev/mapper/vg_bp05-lv_home
 001      /mnt/test2          /dev/sdc1

why did that work?

since we're always loading *all* paths in the table now, whatever checks
the limit is looking at fs_count not xfs_fs_count.  Sigh.

So something like this (need to add xfs_fs_count to path.h too)

diff --git a/quota/path.c b/quota/path.c
index bb28d82..546db52 100644
--- a/quota/path.c
+++ b/quota/path.c
@@ -105,6 +105,7 @@ path_f(
        char            **argv)
 {
        int             i;
+       int             max = foreign_allowed ? fs_count : xfs_fs_count;
 
        if (fs_count == 0) {
                printf(_("No paths are available\n"));
@@ -115,9 +116,9 @@ path_f(
                return pathlist_f();
 
        i = atoi(argv[1]);
-       if (i < 0 || i >= fs_count) {
+       if (i < 0 || i >= max) {
                printf(_("value %d is out of range (0-%d)\n"),
-                       i, fs_count-1);
+                       i, max - 1);
        } else {
                fs_path = &fs_table[i];
                pathlist_f();

and probably need to audit for any other use of "fs_count" ... hohum.
init_args_command for example, whatever the hell that thing does ;)

-Eric

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

Re: [PATCH 1/3] xfsprogs: populate fs table with xfs entries first, foreign entries last

[Eric Sandeen]

On 9/14/16 12:50 PM, Eric Sandeen wrote:
>> @@ -113,8 +119,13 @@ path_f(
> Oops, I mentioned this to you as needed, but it's wrong, sorry.
> 
> Just drop this hunk, this is supposed to select path i, not iterate
> over anything.  Sorry for mentioning it.
> 
>> >  		printf(_("value %d is out of range (0-%d)\n"),
>> >  			i, fs_count-1);
>> >  	} else {
>> > -		fs_path = &fs_table[i];
>> > -		pathlist_f();
>> > +		for (i = 0; i < fs_count; i++) {
>> > +			fs_path = &fs_table[i];
>> > +			/* Table is ordered xfs first, then foreign */
>> > +			if (fs_path->fs_flags & FS_FOREIGN && !foreign_allowed)
>> > +				break;
>> > +			pathlist_f();
>> > +		}
>> >  	}
>> >  	return 0;
>> >  }

Oh, right.  You need the test & break in print_f not path_f, otherwise:

# quota/xfs_quota -x
xfs_quota> print
Filesystem          Pathname
    /home               /dev/mapper/vg_bp05-lv_home
    /mnt/test2          /dev/sdc1
(F) /                   /dev/mapper/vg_bp05-lv_root
(F) /boot               /dev/sda1
(F) /mnt/test           /dev/sdb1
xfs_quota> 

"print" gives you foreign filesystems when you didn't ask for them.

-Eric

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

Re: [PATCH 2/3] xfs_quota: print and path output formatting: maintain reverse compatibility

[Eric Sandeen]

On 9/14/16 10:19 AM, Bill O'Donnell wrote:
> This patch adjusts the formatting of the xfs_quota print and
> path outputs, in order to maintain reverse compatability:
> when -f flag isn't used, need to keep the output same as in
> previous version.
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> ---
>  quota/path.c | 67 +++++++++++++++++++++++++++++++++++-------------------------
>  1 file changed, 39 insertions(+), 28 deletions(-)
> 
> diff --git a/quota/path.c b/quota/path.c
> index aa3d33e..ed9c044 100644
> --- a/quota/path.c
> +++ b/quota/path.c
> @@ -36,39 +36,50 @@ printpath(
>  	int		c;
>  
>  	if (index == 0) {
> -		printf(_("%sFilesystem          Pathname\n"),
> -			number ? _("      ") : "");
> +		if (foreign_allowed)
> +			printf(_("%s    Filesystem          Pathname\n"),
> +			       number ? _("      ") : "");
> +		else
> +			printf(_("%sFilesystem          Pathname\n"),
> +			       number ? _("      ") : "");

Ok, this prints the header, with differing leading whitespace depending on
foreign and/or "are we printing the path number?"  It works but it seems
confusing to me.

What about just being explicit about it, something like:

+       /* print header, accommodating for path nr and/or foreign flag */
  	if (index == 0) {
+               if (number)
+                       printf(_("      "));
+               if (foreign_allowed)
+                       printf(_("    "));
+               printf(_("Filesystem          Pathname\n"));

or maybe:

  	if (index == 0) {
+		printf(_("%s%sFilesystem          Pathname\n"),
+			number ? _("      ") : "",
+			foreign_allowed ? _("    ") : "");

to follow the existing code, just with another "optional" column for (F)?

>  	}
>  	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);
> -		printf(_(" (project %u"), path->fs_prid);
> -		if (prj)
> -			printf(_(", %s"), prj->pr_name);
> -		printf(")");
> -	} else if (xfsquotactl(XFS_GETQSTAT, path->fs_name, 0, 0,
> -				(void *)&qstat) == 0 && qstat.qs_flags) {
> -		c = 0;
> -		printf(" (");
> -		if (qstat.qs_flags & XFS_QUOTA_UDQ_ENFD)
> -			c = printf("uquota");
> -		else if (qstat.qs_flags & XFS_QUOTA_UDQ_ACCT)
> -			c = printf("uqnoenforce");
> -		if (qstat.qs_flags & XFS_QUOTA_GDQ_ENFD)
> -			c = printf("%sgquota", c ? ", " : "");
> -		else if (qstat.qs_flags & XFS_QUOTA_GDQ_ACCT)
> -			c = printf("%sgqnoenforce", c ? ", " : "");
> -		if (qstat.qs_flags & XFS_QUOTA_PDQ_ENFD)
> -			printf("%spquota", c ? ", " : "");
> -		else if (qstat.qs_flags & XFS_QUOTA_PDQ_ACCT)
> -			printf("%spqnoenforce", c ? ", " : "");
> -		printf(")");

Ok below is mostly just moving indentation under this:

> +	if (!((path->fs_flags & FS_FOREIGN) && !foreign_allowed)) {

a big conditional, which I think means "If it's not a foreign filesystem
with foreign filesystems disallowed..."

Can we ever get her when that's not true?  Do we ever call into this
with a foreign fs when !foreign_allowed?  I don't think so, because we
break in the pathlist_f caller, right?  and in print_f ... oh right, need
to break out of print_f, too.  See my latest reply to patch 1.

With that, should not need to move any of this code under the conditional;
it can go away and the code can remain where it's at.

> +		printf("%s", (path->fs_flags & FS_FOREIGN) ? "(F) " : "");
> +		if (path->fs_flags & FS_FOREIGN)
> +			printf(_("%-19s %s"), path->fs_dir, path->fs_name);
> +		else if (foreign_allowed)
> +			printf(_("    %-19s %s"), path->fs_dir, path->fs_name);
> +		else
> +			printf(_("%-19s %s"), path->fs_dir, path->fs_name);

Hm, above you've got 3 cases for 2 different printf strings.  ;)  This would
be much simpler as:

 	if (number) {
  		printf(_("%c%03d%c "), braces? '[':' ', index, braces? ']':' ');
+       if (foreign_allowed)
+               printf("%s", path->fs_flags & FS_FOREIGN ? "(F) " : "    ");
+
        printf(_("%-19s %s"), path->fs_dir, path->fs_name);

So if we're printing numbers, print that column; if we're dealing with foreign FS's,
print that column, and then print the common information in the last 2 columns.

> +		if (path->fs_flags & FS_PROJECT_PATH) {
> +			prj = getprprid(path->fs_prid);
> +			printf(_(" (project %u"), path->fs_prid);
> +			if (prj)
> +				printf(_(", %s"), prj->pr_name);
> +			printf(")");
> +		} else if (xfsquotactl(XFS_GETQSTAT, path->fs_name, 0, 0,
> +				       (void *)&qstat) == 0 && qstat.qs_flags) {
> +			c = 0;
> +			printf(" (");
> +			if (qstat.qs_flags & XFS_QUOTA_UDQ_ENFD)
> +				c = printf("uquota");
> +			else if (qstat.qs_flags & XFS_QUOTA_UDQ_ACCT)
> +				c = printf("uqnoenforce");
> +			if (qstat.qs_flags & XFS_QUOTA_GDQ_ENFD)
> +				c = printf("%sgquota", c ? ", " : "");
> +			else if (qstat.qs_flags & XFS_QUOTA_GDQ_ACCT)
> +				c = printf("%sgqnoenforce", c ? ", " : "");
> +			if (qstat.qs_flags & XFS_QUOTA_PDQ_ENFD)
> +				printf("%spquota", c ? ", " : "");
> +			else if (qstat.qs_flags & XFS_QUOTA_PDQ_ACCT)
> +				printf("%spqnoenforce", c ? ", " : "");
> +			printf(")");
> +		}
> +		printf("\n");

and again w/o the conditional this whole indentation move goes away, and
the patch looks much simpler overall :)  -

diff --git a/quota/path.c b/quota/path.c
index cf00fc5..622c6b5 100644
--- a/quota/path.c
+++ b/quota/path.c
@@ -36,14 +36,19 @@ printpath(
        int             c;
 
        if (index == 0) {
-               printf(_("%sFilesystem          Pathname\n"),
-                       number ? _("      ") : "");
+               printf(_("%s%sFilesystem          Pathname\n"),
+                       number ? _("      ") : "",
+                       foreign_allowed ? _("    ") : "");
        }
-       if (number) {
+
+       if (number)
                printf(_("%c%03d%c "), braces? '[':' ', index, braces? ']':' ');
-       }
-       printf("%s ", (path->fs_flags & FS_FOREIGN) ? "(F)" : "   ");
+
+       if (foreign_allowed)
+               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);
                printf(_(" (project %u"), path->fs_prid);



(but don't take my word for it!  ;)  I think this works though.)

[PATCH v2 0/3] xfsprogs: xfs_quota foreign fs path handling modifications

[Bill O'Donnell]

Hello-

This is version 2 of a series that supercedes:
http://www.spinics.net/lists/xfs/msg42220.html -
(xfs_quota foreign fs path handling changes).

The following commits modified xfs_quota for use on foreign
filesystems:
b20b6c2 xfs_quota: certain commands must always be available
29647c8 xfs_quota: add capabilities for use on non-XFS filesystems

Included in those commits were modifications to
fs_initialise_mounts (paths.c), resulting in a reverse
compatibility issue. In particular, xfstest xfs/261 failed,
due to foreign fs paths being picked up from the fs table,
and the xfs_quota print command complaining about not being
able to print the foreign paths.

This series fixes the foreign path print problem, via
modifications to the xfs_quota paths and print operations.

patch 1: populate the fs table with xfs entries first,
maintaining mtab order, followed by foreign fs entries.

patch 2: adjust print and path formatting of xfs and
foreign fs entries, maintaining reverse compatibility.

patch 3: add the case for foreign fs in init_check_command
to cover both states of the -f (foreign_allowed) flag.

------
history:
v2: patch 1: -Improve commit message.
             -Align xfs_fs_count declaration and add it to path.h.
             -Correct src and dest initializations in memmove
              in fs_table_insert (fix valgrind error).
             -Remove unnecessary iterative loop in path_f.
             -Add iterative loop in print_f (break if foreign fs
              encountered and !foreign_allowed).
             -Correct path_f to use xfs_fs_count when !foreign_allowed.

    patch 2: -Simplify formatting logic for print and path.

    patch 3: -no changes.

v1: http://www.spinics.net/lists/xfs/msg42399.html
------

Again, Questions/comments are welcome.

Thanks-
Bill

[PATCH v2 1/3] xfsprogs: populate fs table with xfs entries first, foreign entries last

[Bill O'Donnell]

Commits b20b6c2 and 29647c8 modified xfs_quota for use on
non-XFS filesystems. Modifications to fs_initialise_mounts
(paths.c) resulted in an xfstest fail (xfs/261), due to foreign
fs paths being picked up first from the fs table. The xfs_quota
print command then complained about not being able to print the
foreign paths, instead of previous behavior (quiet).

This patch restores correct behavior, sorting the table so that
xfs entries are first, followed by foreign fs entries. The patch
maintains the order of xfs entries and foreign entries in the
same order as mtab entries. Then, in functions which print all
paths we can simply break at the first foreign path if the -f
switch is not specified.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 include/path.h  |  1 +
 libxcmd/paths.c | 12 +++++++++++-
 quota/path.c    | 24 ++++++++++++++++++------
 3 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/include/path.h b/include/path.h
index 39c1a95..d077bac 100644
--- a/include/path.h
+++ b/include/path.h
@@ -44,6 +44,7 @@ typedef struct fs_path {
 } fs_path_t;
 
 extern int fs_count;		/* number of entries in fs table */
+extern int xfs_fs_count;	/* number of xfs entries in fs table */
 extern fs_path_t *fs_table;	/* array of entries in fs table  */
 extern fs_path_t *fs_path;	/* current entry in the fs table */
 extern char *mtab_file;
diff --git a/libxcmd/paths.c b/libxcmd/paths.c
index 4158688..3455217 100644
--- a/libxcmd/paths.c
+++ b/libxcmd/paths.c
@@ -32,6 +32,7 @@
 extern char *progname;
 
 int fs_count;
+int xfs_fs_count;
 struct fs_path *fs_table;
 struct fs_path *fs_path;
 
@@ -138,7 +139,16 @@ fs_table_insert(
 		goto out_norealloc;
 	fs_table = tmp_fs_table;
 
-	fs_path = &fs_table[fs_count];
+	/* Put foreign filesystems at the end, xfs filesystems at the front */
+	if (flags & FS_FOREIGN || fs_count == 0) {
+		fs_path = &fs_table[fs_count];
+	} else {
+		/* move foreign fs entries down, insert new one just before */
+		memmove(&fs_table[xfs_fs_count + 1], &fs_table[xfs_fs_count],
+			sizeof(fs_path_t)*(fs_count - xfs_fs_count));
+		fs_path = &fs_table[xfs_fs_count];
+		xfs_fs_count++;
+	}
 	fs_path->fs_dir = dir;
 	fs_path->fs_prid = prid;
 	fs_path->fs_flags = flags;
diff --git a/quota/path.c b/quota/path.c
index a623d25..01ccab4 100644
--- a/quota/path.c
+++ b/quota/path.c
@@ -75,9 +75,15 @@ static int
 pathlist_f(void)
 {
 	int		i;
+	struct fs_path	*path;
 
-	for (i = 0; i < fs_count; i++)
-		printpath(&fs_table[i], i, 1, &fs_table[i] == fs_path);
+	for (i = 0; i < fs_count; i++) {
+		path = &fs_table[i];
+		/* Table is ordered xfs first, then foreign */
+		if (path->fs_flags & FS_FOREIGN && !foreign_allowed)
+			break;
+		printpath(path, i, 1, path == fs_path);
+	}
 	return 0;
 }
 
@@ -87,9 +93,14 @@ print_f(
 	char		**argv)
 {
 	int		i;
+	struct fs_path	*path;
 
-	for (i = 0; i < fs_count; i++)
-		printpath(&fs_table[i], i, 0, 0);
+	for (i = 0; i < fs_count; i++) {
+		path = &fs_table[i];
+		if (path->fs_flags & FS_FOREIGN && !foreign_allowed)
+			break;
+		printpath(path, i, 0, 0);
+	}
 	return 0;
 }
 
@@ -99,6 +110,7 @@ path_f(
 	char		**argv)
 {
 	int	i;
+	int	max = foreign_allowed ? fs_count : xfs_fs_count;
 
 	if (fs_count == 0) {
 		printf(_("No paths are available\n"));
@@ -109,9 +121,9 @@ path_f(
 		return pathlist_f();
 
 	i = atoi(argv[1]);
-	if (i < 0 || i >= fs_count) {
+	if (i < 0 || i >= max) {
 		printf(_("value %d is out of range (0-%d)\n"),
-			i, fs_count-1);
+			i, max - 1);
 	} else {
 		fs_path = &fs_table[i];
 		pathlist_f();
-- 
2.7.4

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

[PATCH v2 2/3] xfs_quota: print and path output formatting: maintain reverse compatibility

[Bill O'Donnell]

This patch adjusts the formatting of the xfs_quota print and
path outputs, in order to maintain reverse compatability:
when -f flag isn't used, need to keep the output same as in
previous version.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 quota/path.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/quota/path.c b/quota/path.c
index 01ccab4..57d14f0 100644
--- a/quota/path.c
+++ b/quota/path.c
@@ -36,13 +36,14 @@ printpath(
 	int		c;
 
 	if (index == 0) {
-		printf(_("%sFilesystem          Pathname\n"),
-			number ? _("      ") : "");
+		printf(_("%s%sFilesystem          Pathname\n"),
+		       number ? _("      ") : "",
+		       foreign_allowed ? _("    ") : "");
 	}
-	if (number) {
+	if (number)
 		printf(_("%c%03d%c "), braces? '[':' ', index, braces? ']':' ');
-	}
-	printf("%s ", (path->fs_flags & FS_FOREIGN) ? "(F)" : "   ");
+	if (foreign_allowed)
+		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);
@@ -51,7 +52,7 @@ printpath(
 			printf(_(", %s"), prj->pr_name);
 		printf(")");
 	} else if (xfsquotactl(XFS_GETQSTAT, path->fs_name, 0, 0,
-				(void *)&qstat) == 0 && qstat.qs_flags) {
+			       (void *)&qstat) == 0 && qstat.qs_flags) {
 		c = 0;
 		printf(" (");
 		if (qstat.qs_flags & XFS_QUOTA_UDQ_ENFD)
-- 
2.7.4

[PATCH v2 3/3] xfs_quota: add case for foreign fs, disabled regardless of foreign_allowed

[Bill O'Donnell]

Some commands are disallowed for foreign filesystems,
regardless of whether or not the -f flag is thrown.
Add a case for this condition and improve commenting
and output messaging accordingly in init_check_command.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 quota/init.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/quota/init.c b/quota/init.c
index 2c18c8b..7d69663 100644
--- a/quota/init.c
+++ b/quota/init.c
@@ -112,21 +112,28 @@ init_check_command(
 	if (!fs_path)
 		return 1;
 
-	/* Always run commands that we are told to skip here */
+	/* Always run commands that are valid for all fs types. */
 	if (ct->flags & CMD_ALL_FSTYPES)
 		return 1;
 
-	/* if it's an XFS filesystem, always run the command */
+	/* 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 the user specified foreign filesystems are ok (-f), run cmd. */
 	if (foreign_allowed &&
 	    (ct->flags & CMD_FLAG_FOREIGN_OK))
 		return 1;
 
-	/* foreign filesystem and it's not a valid command! */
-	fprintf(stderr, _("%s command is for XFS filesystems only\n"),
+	/* If cmd not allowed on foreign fs, regardless of -f flag, skip it. */
+	if (!(ct->flags & CMD_FLAG_FOREIGN_OK)) {
+		fprintf(stderr, _("%s: command is for XFS filesystems only\n"),
+			ct->name);
+		return 0;
+	}
+
+	/* foreign fs, but cmd only allowed via -f flag. Skip it. */
+	fprintf(stderr, _("%s: foreign filesystem. Use -f to enable.\n"),
 		ct->name);
 	return 0;
 }
-- 
2.7.4

Re: [PATCH v2 1/3] xfsprogs: populate fs table with xfs entries first, foreign entries last

[Eric Sandeen]

On 9/15/16 10:29 AM, Bill O'Donnell wrote:
> Commits b20b6c2 and 29647c8 modified xfs_quota for use on
> non-XFS filesystems. Modifications to fs_initialise_mounts
> (paths.c) resulted in an xfstest fail (xfs/261), due to foreign
> fs paths being picked up first from the fs table. The xfs_quota
> print command then complained about not being able to print the
> foreign paths, instead of previous behavior (quiet).
> 
> This patch restores correct behavior, sorting the table so that
> xfs entries are first, followed by foreign fs entries. The patch
> maintains the order of xfs entries and foreign entries in the
> same order as mtab entries. Then, in functions which print all
> paths we can simply break at the first foreign path if the -f
> switch is not specified.
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>

worksforme, thanks

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  include/path.h  |  1 +
>  libxcmd/paths.c | 12 +++++++++++-
>  quota/path.c    | 24 ++++++++++++++++++------
>  3 files changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/include/path.h b/include/path.h
> index 39c1a95..d077bac 100644
> --- a/include/path.h
> +++ b/include/path.h
> @@ -44,6 +44,7 @@ typedef struct fs_path {
>  } fs_path_t;
>  
>  extern int fs_count;		/* number of entries in fs table */
> +extern int xfs_fs_count;	/* number of xfs entries in fs table */
>  extern fs_path_t *fs_table;	/* array of entries in fs table  */
>  extern fs_path_t *fs_path;	/* current entry in the fs table */
>  extern char *mtab_file;
> diff --git a/libxcmd/paths.c b/libxcmd/paths.c
> index 4158688..3455217 100644
> --- a/libxcmd/paths.c
> +++ b/libxcmd/paths.c
> @@ -32,6 +32,7 @@
>  extern char *progname;
>  
>  int fs_count;
> +int xfs_fs_count;
>  struct fs_path *fs_table;
>  struct fs_path *fs_path;
>  
> @@ -138,7 +139,16 @@ fs_table_insert(
>  		goto out_norealloc;
>  	fs_table = tmp_fs_table;
>  
> -	fs_path = &fs_table[fs_count];
> +	/* Put foreign filesystems at the end, xfs filesystems at the front */
> +	if (flags & FS_FOREIGN || fs_count == 0) {
> +		fs_path = &fs_table[fs_count];
> +	} else {
> +		/* move foreign fs entries down, insert new one just before */
> +		memmove(&fs_table[xfs_fs_count + 1], &fs_table[xfs_fs_count],
> +			sizeof(fs_path_t)*(fs_count - xfs_fs_count));
> +		fs_path = &fs_table[xfs_fs_count];
> +		xfs_fs_count++;
> +	}
>  	fs_path->fs_dir = dir;
>  	fs_path->fs_prid = prid;
>  	fs_path->fs_flags = flags;
> diff --git a/quota/path.c b/quota/path.c
> index a623d25..01ccab4 100644
> --- a/quota/path.c
> +++ b/quota/path.c
> @@ -75,9 +75,15 @@ static int
>  pathlist_f(void)
>  {
>  	int		i;
> +	struct fs_path	*path;
>  
> -	for (i = 0; i < fs_count; i++)
> -		printpath(&fs_table[i], i, 1, &fs_table[i] == fs_path);
> +	for (i = 0; i < fs_count; i++) {
> +		path = &fs_table[i];
> +		/* Table is ordered xfs first, then foreign */
> +		if (path->fs_flags & FS_FOREIGN && !foreign_allowed)
> +			break;
> +		printpath(path, i, 1, path == fs_path);
> +	}
>  	return 0;
>  }
>  
> @@ -87,9 +93,14 @@ print_f(
>  	char		**argv)
>  {
>  	int		i;
> +	struct fs_path	*path;
>  
> -	for (i = 0; i < fs_count; i++)
> -		printpath(&fs_table[i], i, 0, 0);
> +	for (i = 0; i < fs_count; i++) {
> +		path = &fs_table[i];
> +		if (path->fs_flags & FS_FOREIGN && !foreign_allowed)
> +			break;
> +		printpath(path, i, 0, 0);
> +	}
>  	return 0;
>  }
>  
> @@ -99,6 +110,7 @@ path_f(
>  	char		**argv)
>  {
>  	int	i;
> +	int	max = foreign_allowed ? fs_count : xfs_fs_count;
>  
>  	if (fs_count == 0) {
>  		printf(_("No paths are available\n"));
> @@ -109,9 +121,9 @@ path_f(
>  		return pathlist_f();
>  
>  	i = atoi(argv[1]);
> -	if (i < 0 || i >= fs_count) {
> +	if (i < 0 || i >= max) {
>  		printf(_("value %d is out of range (0-%d)\n"),
> -			i, fs_count-1);
> +			i, max - 1);
>  	} else {
>  		fs_path = &fs_table[i];
>  		pathlist_f();
> 

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

Re: [PATCH v2 2/3] xfs_quota: print and path output formatting: maintain reverse compatibility

[Eric Sandeen]

On 9/15/16 10:29 AM, Bill O'Donnell wrote:
> This patch adjusts the formatting of the xfs_quota print and
> path outputs, in order to maintain reverse compatability:
> when -f flag isn't used, need to keep the output same as in
> previous version.
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>

Last hunk ended up being a bit extraneous (unrelated whitespace
changes) but *shrug* Dave could drop it on commit maybe, if not
no biggie.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

Thanks,
-Eric

> ---
>  quota/path.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/quota/path.c b/quota/path.c
> index 01ccab4..57d14f0 100644
> --- a/quota/path.c
> +++ b/quota/path.c
> @@ -36,13 +36,14 @@ printpath(
>  	int		c;
>  
>  	if (index == 0) {
> -		printf(_("%sFilesystem          Pathname\n"),
> -			number ? _("      ") : "");
> +		printf(_("%s%sFilesystem          Pathname\n"),
> +		       number ? _("      ") : "",
> +		       foreign_allowed ? _("    ") : "");
>  	}
> -	if (number) {
> +	if (number)
>  		printf(_("%c%03d%c "), braces? '[':' ', index, braces? ']':' ');
> -	}
> -	printf("%s ", (path->fs_flags & FS_FOREIGN) ? "(F)" : "   ");
> +	if (foreign_allowed)
> +		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);
> @@ -51,7 +52,7 @@ printpath(
>  			printf(_(", %s"), prj->pr_name);
>  		printf(")");
>  	} else if (xfsquotactl(XFS_GETQSTAT, path->fs_name, 0, 0,
> -				(void *)&qstat) == 0 && qstat.qs_flags) {
> +			       (void *)&qstat) == 0 && qstat.qs_flags) {
>  		c = 0;
>  		printf(" (");
>  		if (qstat.qs_flags & XFS_QUOTA_UDQ_ENFD)
> 

Re: [PATCH v2 3/3] xfs_quota: add case for foreign fs, disabled regardless of foreign_allowed

[Eric Sandeen]

On 9/15/16 10:29 AM, Bill O'Donnell wrote:
> Some commands are disallowed for foreign filesystems,
> regardless of whether or not the -f flag is thrown.
> Add a case for this condition and improve commenting
> and output messaging accordingly in init_check_command.
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>

This looks good, though as I mentioned on IRC, 

# quota/xfs_quota -x /dev/sdb1 
xfs_quota> dump
dump: foreign filesystem. Use -f to enable.
xfs_quota> dump -f
dump: foreign filesystem. Use -f to enable.

that specific wording might be confusing.  :)

I think maybe:

-	fprintf(stderr, _("%s: foreign filesystem. Use -f to enable.\n"),
+	fprintf(stderr, _("%s: foreign filesystem. Invoke xfs_quota with -f to enable.\n"),

would be better - can you resend with that small change?

-Eric

> ---
>  quota/init.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/quota/init.c b/quota/init.c
> index 2c18c8b..7d69663 100644
> --- a/quota/init.c
> +++ b/quota/init.c
> @@ -112,21 +112,28 @@ init_check_command(
>  	if (!fs_path)
>  		return 1;
>  
> -	/* Always run commands that we are told to skip here */
> +	/* Always run commands that are valid for all fs types. */
>  	if (ct->flags & CMD_ALL_FSTYPES)
>  		return 1;
>  
> -	/* if it's an XFS filesystem, always run the command */
> +	/* 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 the user specified foreign filesystems are ok (-f), run cmd. */
>  	if (foreign_allowed &&
>  	    (ct->flags & CMD_FLAG_FOREIGN_OK))
>  		return 1;
>  
> -	/* foreign filesystem and it's not a valid command! */
> -	fprintf(stderr, _("%s command is for XFS filesystems only\n"),
> +	/* If cmd not allowed on foreign fs, regardless of -f flag, skip it. */
> +	if (!(ct->flags & CMD_FLAG_FOREIGN_OK)) {
> +		fprintf(stderr, _("%s: command is for XFS filesystems only\n"),
> +			ct->name);
> +		return 0;
> +	}
> +
> +	/* foreign fs, but cmd only allowed via -f flag. Skip it. */
> +	fprintf(stderr, _("%s: foreign filesystem. Use -f to enable.\n"),
>  		ct->name);
>  	return 0;
>  }
> 

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

[PATCH v3 3/3] xfs_quota: add case for foreign fs, disabled regardless of foreign_allowed

[Bill O'Donnell]

Some commands are disallowed for foreign filesystems,
regardless of whether or not the -f flag is thrown.
Add a case for this condition and improve commenting
and output messaging accordingly in init_check_command.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
history:
v3: clarify foreign filesystem user output message.
v2: no change
v1: http://www.spinics.net/lists/linux-xfs/msg00685.html

 quota/init.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/quota/init.c b/quota/init.c
index 2c18c8b..3bebbb8 100644
--- a/quota/init.c
+++ b/quota/init.c
@@ -112,21 +112,29 @@ init_check_command(
 	if (!fs_path)
 		return 1;
 
-	/* Always run commands that we are told to skip here */
+	/* Always run commands that are valid for all fs types. */
 	if (ct->flags & CMD_ALL_FSTYPES)
 		return 1;
 
-	/* if it's an XFS filesystem, always run the command */
+	/* 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 the user specified foreign filesystems are ok (-f), run cmd. */
 	if (foreign_allowed &&
 	    (ct->flags & CMD_FLAG_FOREIGN_OK))
 		return 1;
 
-	/* foreign filesystem and it's not a valid command! */
-	fprintf(stderr, _("%s command is for XFS filesystems only\n"),
+	/* If cmd not allowed on foreign fs, regardless of -f flag, skip it. */
+	if (!(ct->flags & CMD_FLAG_FOREIGN_OK)) {
+		fprintf(stderr, _("%s: command is for XFS filesystems only\n"),
+			ct->name);
+		return 0;
+	}
+
+	/* foreign fs, but cmd only allowed via -f flag. Skip it. */
+	fprintf(stderr,
+		_("%s: foreign filesystem. Invoke xfs_quota with -f to enable.\n"),
 		ct->name);
 	return 0;
 }
-- 
2.7.4

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

Re: [PATCH v3 3/3] xfs_quota: add case for foreign fs, disabled regardless of foreign_allowed

[Eric Sandeen]

On 9/15/16 2:57 PM, Bill O'Donnell wrote:
> Some commands are disallowed for foreign filesystems,
> regardless of whether or not the -f flag is thrown.
> Add a case for this condition and improve commenting
> and output messaging accordingly in init_check_command.
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>

Thanks, Bill.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
> history:
> v3: clarify foreign filesystem user output message.
> v2: no change
> v1: http://www.spinics.net/lists/linux-xfs/msg00685.html
> 
>  quota/init.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/quota/init.c b/quota/init.c
> index 2c18c8b..3bebbb8 100644
> --- a/quota/init.c
> +++ b/quota/init.c
> @@ -112,21 +112,29 @@ init_check_command(
>  	if (!fs_path)
>  		return 1;
>  
> -	/* Always run commands that we are told to skip here */
> +	/* Always run commands that are valid for all fs types. */
>  	if (ct->flags & CMD_ALL_FSTYPES)
>  		return 1;
>  
> -	/* if it's an XFS filesystem, always run the command */
> +	/* 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 the user specified foreign filesystems are ok (-f), run cmd. */
>  	if (foreign_allowed &&
>  	    (ct->flags & CMD_FLAG_FOREIGN_OK))
>  		return 1;
>  
> -	/* foreign filesystem and it's not a valid command! */
> -	fprintf(stderr, _("%s command is for XFS filesystems only\n"),
> +	/* If cmd not allowed on foreign fs, regardless of -f flag, skip it. */
> +	if (!(ct->flags & CMD_FLAG_FOREIGN_OK)) {
> +		fprintf(stderr, _("%s: command is for XFS filesystems only\n"),
> +			ct->name);
> +		return 0;
> +	}
> +
> +	/* foreign fs, but cmd only allowed via -f flag. Skip it. */
> +	fprintf(stderr,
> +		_("%s: foreign filesystem. Invoke xfs_quota with -f to enable.\n"),
>  		ct->name);
>  	return 0;
>  }
>