On 5/17/16 10:08 PM, Zorro Lang wrote: > 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. Yep :) But it only messes with "day" reporting, not full-resolution reporting, which was why I sent that commit in the first place. I just missed that it was used for sane "set+query" behavior, until you pointed out the difference. > 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). Well, at some point it will change as time goes by, of course. Unfortunately IIRC I couldn't find the reason for the original commit; it's been that way for a very longtime. I just had to infer that it was so you could set & query and get back the "days" value that was just set. > 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. Eh, I think this is getting too much into the details. The user doesn't need to know all the details of every single programming decision. :) -Eric > 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 > _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
