diff mbox

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

Message ID 1473953386-10242-4-git-send-email-billodo@redhat.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Bill O'Donnell Sept. 15, 2016, 3:29 p.m. UTC
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(-)

Comments

Eric Sandeen Sept. 15, 2016, 7:32 p.m. UTC | #1
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;
>  }
>
diff mbox

Patch

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;
 }