[libvirt] [PATCH v2 1/2] timer impl

[Wen Congyang]

At 12/15/2010 11:20 PM, Eric Blake Write:
> On 12/14/2010 07:34 PM, Wen Congyang wrote:
> 
> In addition to Hu's comments, and the fact that you are probably going
> to revise the exposed interface anyways, here's some additional points.
> 
>> * src/util/timer.c src/util/timer.h src/util/timer_linux.c src/util/timer_win32.c:
>>   timer implementation
>> * src/Makefile.am: build timer
>> * src/libvirt_private.syms: Export public functions
>> * src/libvirt.c: Initialize timer
>> * configure.ac: check the functions in librt used by timer
>>
>> Signed-off-by: Wen Congyang <wency at cn.fujitsu.com>
>>
>>  
>> -EXTRA_DIST += util/threads-pthread.c util/threads-win32.c
>> +EXTRA_DIST += util/threads-pthread.c util/threads-win32.c \
>> +		util/timer_linux.c
> 
> timer-win32.c?  Also, I'd go with timer-linux.c, not timer_linux.c.
> 
>> +# timer.h
>> +get_clock;
> 
> Bad idea to pollute the namespace with get_clock; better would be
> something like virGetClock.
> 
>> +virNewTimer;
>> +virFreeTimer;
>> +virModTimer;
>> +virDelTimer;
>>  
>>  # usb.h
>> +
>> +static virTimerPtr timer_list = NULL;
>> +static void realarm_timer(void);
>> +static void __realarm_timer(uint64_t);
> 
> It is dangerous to declare functions in the __ namespace, since that is
> reserved for libc and friends.
> 
>> +uint64_t get_clock(void)
>> +{
>> +    struct timespec ts;
>> +
>> +    clock_gettime(CLOCK_MONOTONIC, &ts);
>> +    return ts.tv_sec * 1000000000ULL + ts.tv_nsec;
> 
> You probably ought to check for overflow here.  Dealing with raw
> nanoseconds is rather fine-grained; is it any better to go with micro or
> even milliseconds, or does libvirt really require something as precise
> as nanosecond timeouts?
> 
Thanks for your comment.

ts.tv_sec * 1000000000ULL will overflow only when the host OS runs 585 years...
So it almost dose not overflow.

I think we do not require nanosecond accuracy. millisecond accuracy is
good enough.