Re: [PATCH] xfs_quota: only round up timer reporting > 1 day

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, May 17, 2016 at 02:36:29PM -0500, Eric Sandeen wrote:
> I was too hasty with:
> 
> d1fe6ff xfs_quota: remove extra 30 seconds from time limit reporting
> 
> The point of that extra 30s, turns out, was to allow the user
> to set a limit, query it, and get back what they just set, if
> it is set to more than a day.
> 
> Without it, if we set a grace period to i.e. 3 days, and query it
> 1 second later, the rounding in the time_to_string function returns
> "2 days" not "3 days" as it did before, because we are at 
> 2 days 23:59:59 and it essentially applies a floor() for
> brevity.  I guess this was confusing.
> 
> (I've run into this same conundrum on my stove digital timer;
> if you set it to 10m, it blinks "10" at you twice so that you
> know what you set, then quickly flips to 9 as it counts down).
> 
> In some cases, however (and this is the case that prompted the
> prior patch), we display a full "XYZ days hh:mm:ss" - we do this
> if the verbose flag is set, or if the timer is less than one day.
> In these cases, we should not add the 30s, because we are showing
> full time resolution to the user.
> 
> Reported-by: Zorro Lang <zlang@xxxxxxxxxx>
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
> 
> (I don't like my patch title, but I can't think of a better one)
> 
> diff --git a/quota/util.c b/quota/util.c
> index 7c43fbd..5e988f9 100644
> --- a/quota/util.c
> +++ b/quota/util.c
> @@ -43,6 +43,18 @@ time_to_string(
>  		timer = MAX(origin - now, 0);
>  	}
>  
> +	/*
> +	 * If we are in verbose mode, or if less than a day remains, we
> +	 * will show "X days hh:mm:ss" so the user knows the exact timer status.
> +	 *
> +	 * Otherwise, we round down to the nearest day - so we add 30s here
> +	 * such that setting and reporting a limit in rapid succession will
> +	 * show the limit which was just set, rather than immediately reporting
> +	 * one day less.
> +	 */
> +	if ((timer > SECONDS_IN_A_DAY) && !(flags & VERBOSE_FLAG))
> +		timer += 30;	/* seconds */
> +

hmm... you nearly bring "d1fe6ff" back. I don't know if +30s is better, but it
nearly won't effect other things. So looks fine for me:)

I have changed the timer in my case from "3days" to "73h", it can keep print
"3day" in one hour, that's enough for my case run over. I saw some other cases
deal with this problem differently, some cases use a filter likes:

    sed -e "/\[2 days\]/s/2 days/3 days/g"

Even if give more 30s, but maybe someone hope to see 3 days become to 2 days
after sleep 5 seconds(although that's really unnecessary).

So I think the necessary thing is we make a clear decision and tell the "users"
about the timer problem, tell them we will show +30s(or not), tell them the
"second" field is always changing, the output can't accurate to the second(it
always older than the real time). Let them know, and notice that, and deal with
it by themselves if they need.

Let the documented problem become a known feature, or we maybe always need
to explain it for someone who feel the timer is not correct:)

What do you think?

Thanks,
Zorro

>  	days = timer / SECONDS_IN_A_DAY;
>  	if (days)
>  		timer %= SECONDS_IN_A_DAY;
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs



[Index of Archives]     [Linux XFS Devel]     [Linux Filesystem Development]     [Filesystem Testing]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux