replacing gettimeofday with clock_gettime in hw/serial
PATCH 1/1 replacing gettimeofday with clock_gettime in hw/serial
This patch substitutes gettimeofday with clock_gettime in hw/serial.c.
gettimeofday is unsafe because can lead to incorrect behaviors if the user changes the system's date.
---
hw/serial.c +20 -20
@@ -155,8 +155,8 @@ struct SerialState {doesn't kill dom0. Simple token bucket. If we get some actual data from the user, instantly refil the bucket. */ -/* How long it takes to generate a token, in microseconds. */ -#define TOKEN_PERIOD 1000 +/* How long it takes to generate a token, in nanoseconds. */ +#define TOKEN_PERIOD 1000000/* Maximum and initial size of token bucket */ #define TOKENS_MAX 100000
@@ -279,47 +279,47 @@ static void serial_update_parameters(SerialState *s)static void serial_get_token (void ) { -static struct timeval last_refil_time; +static struct timespec last_refil_time;static int started;assert (tokens_avail >=0 );if (!tokens_avail) { -struct timeval delta, now; -int generated; +struct timespec delta, now; +long generated;if (!started) { -gettimeofday (&last_refil_time,NULL ); +clock_gettime (CLOCK_MONOTONIC , &last_refil_time); tokens_avail =TOKENS_MAX ; started =1 ;return ; } retry: -gettimeofday (&now,NULL ); +clock_gettime (CLOCK_MONOTONIC , &now); delta.tv_sec = now.tv_sec - last_refil_time.tv_sec; - delta.tv_usec = now.tv_usec - last_refil_time.tv_usec; -if (delta.tv_usec <0 ) { - delta.tv_usec +=1000000 ; + delta.tv_nsec = now.tv_nsec - last_refil_time.tv_nsec; +if (delta.tv_nsec <0 ) { + delta.tv_nsec +=1000000000 ; delta.tv_sec--; } -assert (delta.tv_usec >=0 && delta.tv_sec >=0 ); -if (delta.tv_usec <TOKEN_PERIOD ) { +assert (delta.tv_nsec >=0 && delta.tv_sec >=0 ); +if (delta.tv_nsec <TOKEN_PERIOD ) {struct timespec ts;/* Wait until at least one token is available. */ - ts.tv_sec =TOKEN_PERIOD /1000000 ; - ts.tv_nsec = (TOKEN_PERIOD %1000000 ) *1000 ; + ts.tv_sec =TOKEN_PERIOD /1000000000 ; + ts.tv_nsec =TOKEN_PERIOD %1000000000 ;while (nanosleep (&ts, &ts) <0 && errno ==EINTR ) ; goto retry; } - generated = (delta.tv_sec *1000000 ) /TOKEN_PERIOD ; + generated = (delta.tv_sec *1000000000 ) /TOKEN_PERIOD ; generated += - ((delta.tv_sec *1000000 ) %TOKEN_PERIOD + delta.tv_usec) /TOKEN_PERIOD ; + ((delta.tv_sec *1000000000 ) %TOKEN_PERIOD + delta.tv_nsec) /TOKEN_PERIOD ;assert (generated >0 ); - last_refil_time.tv_usec += (generated *TOKEN_PERIOD ) %1000000 ; - last_refil_time.tv_sec += last_refil_time.tv_usec /1000000 ; - last_refil_time.tv_usec %=1000000 ; - last_refil_time.tv_sec += (generated *TOKEN_PERIOD ) /1000000 ; + last_refil_time.tv_nsec += (generated *TOKEN_PERIOD ) %1000000000 ; + last_refil_time.tv_sec += last_refil_time.tv_nsec /1000000000 ; + last_refil_time.tv_nsec %=1000000000 ; + last_refil_time.tv_sec += (generated *TOKEN_PERIOD ) /1000000000 ;if (generated >TOKENS_MAX ) generated =TOKENS_MAX ; tokens_avail = generated;
Quoted 4 lines · Stefano Stabellini
This patch substitutes gettimeofday with
clock_gettimeinhw/serial.c.gettimeofday is unsafe because can lead to incorrect behaviors if the user changes the system's date.
This code is just plain wrong to start with. Devices should not depend on host time.
Paul
Quoted 10 lines · Paul Brook
Quoted 5 lines · Stefano Stabellini
This patch substitutes gettimeofday with
clock_gettimeinhw/serial.c.gettimeofday is unsafe because can lead to incorrect behaviors if the user changes the system's date.
This code is just plain wrong to start with. Devices should not depend on host time.
Because host time continues even when the guest is stopped. It should instead be based on the vm_clock.
I don't think it works either. I still have gotten messages in the guest about too many interrupts on the serial port.
Regards,
Anthony Liguori
Quoted 4 lines
Paul
Quoted 14 lines · Anthony Liguori
Quoted 4 lines · Paul Brook
Quoted 2 lines · Stefano Stabellini
This patch substitutes gettimeofday with
clock_gettimeinhw/serial.c.Quoted 8 lines
Quoted 4 lines
gettimeofday is unsafe because can lead to incorrect behaviors if the user
changes the system's date.This code is just plain wrong to start with. Devices should not depend on host time.
Quoted 6 lines
Because host time continues even when the guest is stopped. It should instead be based on the
vm_clock.I don't think it works either. I still have gotten messages in the guest about too many interrupts on the serial port.
I am sorry, I did a mistake: I have just realized that the serial emulation code in qemu and ioemu under xen are so different that this patch doesn't even apply to qemu.
I am so used to send the same patch twice that I haven't realize that until now.
I'll try to merge them, if it makes sense.