Re: [PATCH 1/1 V2] xfs_quota: allow individual timer extension

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Mon, May 18, 2020 at 03:09:30PM -0500, Eric Sandeen wrote:
> The only grace period which can be set via xfs_quota today is for id 0,
> i.e. the default grace period for all users.  However, setting an
> individual grace period is useful; for example:
> 
>  Alice has a soft quota of 100 inodes, and a hard quota of 200 inodes
>  Alice uses 150 inodes, and enters a short grace period
>  Alice really needs to use those 150 inodes past the grace period
>  The administrator extends Alice's grace period until next Monday
> 
> vfs quota users such as ext4 can do this today, with setquota -T
> 
> xfs_quota can now accept an optional user id or name (symmetric with
> how warn limits are specified), in which case that user's grace period
> is extended to expire the given amount of time from now(). 
> 
> To maintain compatibility with old command lines, if none of 
> [-d|id|name] are specified, default limits are set as before.
> 
> (kernelspace requires updates to enable all this as well.)
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> V2: Add comments about only extending if past soft limits
>     Fix typo/mistake checking block hard limits instead of soft
> 
> diff --git a/man/man8/xfs_quota.8 b/man/man8/xfs_quota.8
> index e6fe7cd1..dd0479cd 100644
> --- a/man/man8/xfs_quota.8
> +++ b/man/man8/xfs_quota.8
> @@ -457,14 +457,46 @@ must be specified.
>  .B \-bir
>  ]
>  .I value
> +[
> +.B -d
> +|
> +.I id
> +|
> +.I name
> +]
>  .br
>  Allows the quota enforcement timeout (i.e. the amount of time allowed
>  to pass before the soft limits are enforced as the hard limits) to
>  be modified. The current timeout setting can be displayed using the
>  .B state
> -command. The value argument is a number of seconds, but units of
> -\&'minutes', 'hours', 'days', and 'weeks' are also understood
> +command.
> +.br
> +When setting the default timer via the
> +.B \-d
> +option, or for
> +.B id
> +0, or if no argument is given after
> +.I value
> +the
> +.I value
> +argument is a number of seconds indicating the relative amount of time after
> +soft limits are exceeded, before hard limits are enforced.
> +.br
> +When setting any other individual timer by
> +.I id
> +or
> +.I name,
> +the
> +.I value
> +is the number of seconds from now, at which time the hard limits will be enforced.
> +This allows extending the grace time of an individual user who has exceeded soft
> +limits.
> +.br
> +For
> +.I value,
> +units of \&'minutes', 'hours', 'days', and 'weeks' are also understood
>  (as are their abbreviations 'm', 'h', 'd', and 'w').
> +.br
>  .HP
>  .B warn
>  [
> diff --git a/quota/edit.c b/quota/edit.c
> index 442b608c..5fdb8ce7 100644
> --- a/quota/edit.c
> +++ b/quota/edit.c
> @@ -419,6 +419,7 @@ restore_f(
>  
>  static void
>  set_timer(
> +	uint32_t	id,
>  	uint		type,
>  	uint		mask,
>  	char		*dev,
> @@ -427,14 +428,43 @@ set_timer(
>  	fs_disk_quota_t	d;
>  
>  	memset(&d, 0, sizeof(d));
> +
> +	/*
> +	 * If id is specified we are extending grace time by value
> +	 * Otherwise we are setting the default grace time
> +	 */
> +	if (id) {
> +		time_t	now;
> +
> +		/* Get quota to find out whether user is past soft limits */
> +		if (xfsquotactl(XFS_GETQUOTA, dev, type, id, (void *)&d) < 0) {
> +			exitcode = 1;
> +			fprintf(stderr, _("%s: cannot get quota: %s\n"),
> +					progname, strerror(errno));
> +				return;
> +		}
> +
> +		time(&now);
> +
> +		/* Only set grace time if user is already past soft limit */
> +		if (d.d_blk_softlimit && d.d_bcount > d.d_blk_softlimit)
> +			d.d_btimer = now + value;
> +		if (d.d_ino_softlimit && d.d_icount > d.d_ino_softlimit)
> +			d.d_itimer = now + value;
> +		if (d.d_rtb_softlimit && d.d_rtbcount > d.d_rtb_softlimit)
> +			d.d_rtbtimer = now + value;

Hmm, I /was/ going to complain about 32-bit wraparound, but then
realized that the whole ioctl interface is totally __s32 and needs
y2038 updates and meh.

Someone looking for a project could work on either fixing the xfs quota
ioctls, or the vfs quota ioctls, or both, assuming Arnd didn't already
fix the VFS...

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> +	} else {
> +		d.d_btimer = value;
> +		d.d_itimer = value;
> +		d.d_rtbtimer = value;
> +	}
> +
>  	d.d_version = FS_DQUOT_VERSION;
>  	d.d_flags = type;
>  	d.d_fieldmask = mask;
> -	d.d_itimer = value;
> -	d.d_btimer = value;
> -	d.d_rtbtimer = value;
> +	d.d_id = id;
>  
> -	if (xfsquotactl(XFS_SETQLIM, dev, type, 0, (void *)&d) < 0) {
> +	if (xfsquotactl(XFS_SETQLIM, dev, type, id, (void *)&d) < 0) {
>  		exitcode = 1;
>  		fprintf(stderr, _("%s: cannot set timer: %s\n"),
>  				progname, strerror(errno));
> @@ -447,10 +477,15 @@ timer_f(
>  	char		**argv)
>  {
>  	uint		value;
> -	int		c, type = 0, mask = 0;
> +	char		*name = NULL;
> +	uint32_t	id = 0;
> +	int		c, flags = 0, type = 0, mask = 0;
>  
> -	while ((c = getopt(argc, argv, "bgipru")) != EOF) {
> +	while ((c = getopt(argc, argv, "bdgipru")) != EOF) {
>  		switch (c) {
> +		case 'd':
> +			flags |= DEFAULTS_FLAG;
> +			break;
>  		case 'b':
>  			mask |= FS_DQ_BTIMER;
>  			break;
> @@ -474,23 +509,45 @@ timer_f(
>  		}
>  	}
>  
> -	if (argc != optind + 1)
> +	 /*
> +	 * Older versions of the command did not accept -d|id|name,
> +	 * so in that case we assume we're setting default timer,
> +	 * and the last arg is the timer value.
> +	 *
> +	 * Otherwise, if the defaults flag is set, we expect 1 more arg for
> +	 * timer value ; if not, 2 more args: 1 for value, one for id/name.
> +	 */
> +	if (!(flags & DEFAULTS_FLAG) && (argc == optind + 1)) {
> +		value = cvttime(argv[optind++]);
> +	} else if (flags & DEFAULTS_FLAG) {
> +		if (argc != optind + 1)
> +			return command_usage(&timer_cmd);
> +		value = cvttime(argv[optind++]);
> +	} else if (argc == optind + 2) {
> +		value = cvttime(argv[optind++]);
> +		name = (flags & DEFAULTS_FLAG) ? "0" : argv[optind++];
> +	} else
>  		return command_usage(&timer_cmd);
>  
> -	value = cvttime(argv[optind++]);
>  
> +	/* if none of -bir specified, set them all */
>  	if (!mask)
>  		mask = FS_DQ_TIMER_MASK;
>  
>  	if (!type) {
>  		type = XFS_USER_QUOTA;
>  	} else if (type != XFS_GROUP_QUOTA &&
> -	           type != XFS_PROJ_QUOTA &&
> -	           type != XFS_USER_QUOTA) {
> +		   type != XFS_PROJ_QUOTA &&
> +		   type != XFS_USER_QUOTA) {
>  		return command_usage(&timer_cmd);
>  	}
>  
> -	set_timer(type, mask, fs_path->fs_name, value);
> +	if (name)
> +		id = id_from_string(name, type);
> +
> +	if (id >= 0)
> +		set_timer(id, type, mask, fs_path->fs_name, value);
> +
>  	return 0;
>  }
>  
> @@ -616,9 +673,9 @@ edit_init(void)
>  
>  	timer_cmd.name = "timer";
>  	timer_cmd.cfunc = timer_f;
> -	timer_cmd.argmin = 2;
> +	timer_cmd.argmin = 1;
>  	timer_cmd.argmax = -1;
> -	timer_cmd.args = _("[-bir] [-g|-p|-u] value");
> +	timer_cmd.args = _("[-bir] [-g|-p|-u] value [-d|id|name]");
>  	timer_cmd.oneline = _("set quota enforcement timeouts");
>  	timer_cmd.help = timer_help;
>  	timer_cmd.flags = CMD_FLAG_FOREIGN_OK;
>