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

[Michal Privoznik]

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 at huawei.com>
> Signed-off-by: Zhang Bo <oscar.zhangbo at huawei.com>
> ---
>  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.

Michal