xfs
[Top] [All Lists]

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

To: xfs@xxxxxxxxxxx
Subject: Re: [PATCH v2 3/3] xfs_quota: add case for foreign fs, disabled regardless of foreign_allowed
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Thu, 15 Sep 2016 14:32:57 -0500
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1473953386-10242-4-git-send-email-billodo@xxxxxxxxxx>
References: <1473866381-28975-1-git-send-email-billodo@xxxxxxxxxx> <1473953386-10242-1-git-send-email-billodo@xxxxxxxxxx> <1473953386-10242-4-git-send-email-billodo@xxxxxxxxxx>
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0) Gecko/20100101 Thunderbird/45.3.0
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@xxxxxxxxxx>

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

<Prev in Thread] Current Thread [Next in Thread>