Re: [PATCH v2 3/3] xfs_quota: additional changes to allow use on ext4

[Dave Chinner <david@xxxxxxxxxxxxx>]

On Tue, Aug 16, 2016 at 09:16:38AM -0500, 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_FLAG_GENERIC 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.
> 
> Signed-off-by: Bill O'Donnell <billodo@xxxxxxxxxx>
> ---
>  include/command.h | 1 +
>  libxcmd/help.c    | 3 ++-
>  libxcmd/quit.c    | 3 ++-
>  quota/init.c      | 3 ++-
>  4 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/include/command.h b/include/command.h
> index 81d5a4d..1c2898a 100644
> --- a/include/command.h
> +++ b/include/command.h
> @@ -22,6 +22,7 @@
>  
>  #define CMD_FLAG_GLOBAL              (1<<31) /* don't iterate "args" */
>  #define CMD_FLAG_FOREIGN_OK  (1<<30) /* command not restricted to XFS */
> +#define CMD_FLAG_GENERIC     (1<<29) /* command is generic (help, quit) */

I don't think this belongs in include/command.h - it's an xfs_quota
specific behaviour so I'd put this in quota/init.h:

#define CMD_SKIP_CHECK  (1<<0)  /* command is always run */

> diff --git a/quota/init.c b/quota/init.c
> index d5d80c2..85931bf 100644
> --- a/quota/init.c
> +++ b/quota/init.c
> @@ -104,7 +104,8 @@ init_check_command(
>       const cmdinfo_t *ct)
>  {
>       if (fs_path &&
> -         !(ct->flags & CMD_FLAG_FOREIGN_OK) &&
> +         !((ct->flags & CMD_FLAG_FOREIGN_OK) && foreign_allowed) &&
> +         !(ct->flags & CMD_FLAG_GENERIC) &&
>            (fs_path->fs_flags & FS_FOREIGN)) {

This is now sufficiently confusing that it needs to be reworked to
make the logic clear and easily commented. i.e. something like this:

static int
init_check_command(
        const cmdinfo_t *ct)
{
        if (!fspath)
                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;
}

Cheers,

Dave.

-- 
Dave Chinner
david@xxxxxxxxxxxxx