Re: [libvirt] [PATCH] util: make it more robust to calculate timeout value

On 15.05.2015 08:26, zhang bo wrote: > When we change system clock to years ago, a certain CPU may use up 100% cputime. > The reason is that in function virEventPollCalculateTimeout(), we assign the > unsigned long long result to an INT variable, > *timeout = then - now; // timeout is INT, and then/now are long long > if (*timeout < 0) > *timeout = 0; > there's a chance that variable @then minus variable @now may be a very large number > that overflows INT value expression, then *timeout will be negative and be assigned to 0. > Next the 'poll' in function virEventPollRunOnce() will get into an 'endless' while loop there. > thus, the cpu that virEventPollRunOnce() thread runs on will go up to 100%. > > Although as we discussed before in https://www.redhat.com/archives/libvir-list/2015-May/msg00400.html > it should be prohibited to set-time while other applications are running, but it does > seems to have no harm to make the codes more robust. > > Signed-off-by: Wang Yufei <james.wangyufei(a)huawei.com&gt; > Signed-off-by: Zhang Bo <oscar.zhangbo(a)huawei.com&gt; > --- > src/util/vireventpoll.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c > index ffda206..5f5a149 100644 > --- a/src/util/vireventpoll.c > +++ b/src/util/vireventpoll.c > @@ -357,9 +357,10 @@ static int virEventPollCalculateTimeout(int *timeout) > return -1; > > EVENT_DEBUG("Schedule timeout then=%llu now=%llu", then, now); > - *timeout = then - now; > - if (*timeout < 0) > + if (then < now) > *timeout = 0; > + else > + *timeout = (then - now) & 0x7FFFFFFF; You're trying to make this an unsigned value. What's wrong with plain typecast? > } else { > *timeout = -1; > } > I must say this is ugly. If the system clock is changed, all the timeouts should fire, shouldn't they? Otherwise important events can be missed.