Re: [PATCH v3] xfsprogs: xfs_quota command error message improvement

[Eric Sandeen <sandeen@sandeen.net>]

On 6/24/20 10:46 AM, Bill O'Donnell wrote:
> Make the error messages for rudimentary xfs_quota commands
> (off, enable, disable) more user friendly, instead of the
> terse sys error outputs.
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> ---
> 
> v3: remove EINVAL case from the OFF case
> v2: enable internationalization and capitalize new message strings
> 
>  quota/state.c | 31 +++++++++++++++++++++++++------
>  1 file changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/quota/state.c b/quota/state.c
> index 8f9718f1..76b6ceda 100644
> --- a/quota/state.c
> +++ b/quota/state.c
> @@ -306,8 +306,15 @@ enable_enforcement(
>  		return;
>  	}
>  	dir = mount->fs_name;
> -	if (xfsquotactl(XFS_QUOTAON, dir, type, 0, (void *)&qflags) < 0)
> -		perror("XFS_QUOTAON");
> +	if (xfsquotactl(XFS_QUOTAON, dir, type, 0, (void *)&qflags) < 0) {
> +		if (errno == EEXIST)
> +			fprintf(stderr, _("Quota enforcement already enabled.\n"));
> +		else if (errno == EINVAL)
> +			fprintf(stderr,
> +				_("Can't enable when quotas are off.\n"));
> +		else
> +			perror("XFS_QUOTAON");
> +	}
>  	else if (flags & VERBOSE_FLAG)
>  		state_quotafile_mount(stdout, type, mount, flags);
>  }
> @@ -328,8 +335,16 @@ disable_enforcement(
>  		return;
>  	}
>  	dir = mount->fs_name;
> -	if (xfsquotactl(XFS_QUOTAOFF, dir, type, 0, (void *)&qflags) < 0)
> -		perror("XFS_QUOTAOFF");
> +	if (xfsquotactl(XFS_QUOTAOFF, dir, type, 0, (void *)&qflags) < 0) {
> +		if (errno == EEXIST)
> +			fprintf(stderr,
> +				_("Quota enforcement already disabled.\n"));
> +		else if (errno == EINVAL)
> +			fprintf(stderr,
> +				_("Can't disable when quotas are off.\n"));
> +		else
> +			perror("XFS_QUOTAOFF");
> +	}
>  	else if (flags & VERBOSE_FLAG)
>  		state_quotafile_mount(stdout, type, mount, flags);
>  }
> @@ -350,8 +365,12 @@ quotaoff(
>  		return;
>  	}
>  	dir = mount->fs_name;
> -	if (xfsquotactl(XFS_QUOTAOFF, dir, type, 0, (void *)&qflags) < 0)
> -		perror("XFS_QUOTAOFF");
> +	if (xfsquotactl(XFS_QUOTAOFF, dir, type, 0, (void *)&qflags) < 0) {
> +		if (errno == EEXIST)
> +			fprintf(stderr, _("Quota already off.\n"));
> +		else
> +			perror("XFS_QUOTAOFF");
> +	}

Hm...

# mount -o quota /dev/pmem0p1 /mnt/test
# xfs_quota -x -c "off" /mnt/test
# xfs_quota -x -c "off" /mnt/test
XFS_QUOTAOFF: Function not implemented

If the last quota type is turned off, I guess we get ENOSYS.

Until then, we get EEXIST, as you handle above. i.e. if I mount with all 3 quota types:

# mount -o gquota -o uquota -opquota /dev/pmem0p1 /mnt/test
# xfs_quota -x -c off /mnt/test
# xfs_quota -x -c off /mnt/test
Quota already off.
# xfs_quota -x -c "off -g" /mnt/test
# xfs_quota -x -c "off -g" /mnt/test
Quota already off.
# xfs_quota -x -c "off -p" /mnt/test
# xfs_quota -x -c "off -p" /mnt/test
XFS_QUOTAOFF: Function not implemented

we can see the difference.  So this might need a bit more logic (and probably some
kernel code reading?) to make things consistent.

-Eric