Re: [Qemu-devel] qtest: setitimer() failures on Darwin and illumos

2012-05-29 Thread Stefano Stabellini
On Mon, 28 May 2012, Paolo Bonzini wrote:
 Il 28/05/2012 21:40, Andreas Färber ha scritto:
  I'm seeing qemu-timer.c:unix_rearm_timer()'s setitimer() abort with
  EINVAL during `make check` on both platforms. The value of
  nearest_delta_ns appears to be INT64_MAX. Is this expected? Is it
  possible that this value is too large for it_value on some platforms?
  Apple's man page mentions that as possible reason for EINVAL but doesn't
  describe how to obtain such an upper value, nor of course where in the
  QEMU code base we would need to make adaptions. ;)
  
  Any suggestions?
 
 You shouldn't call the rearm function at all if you get INT64_MAX.  This
 applies to all timers.

Yep. In fact qemu_rearm_alarm_timer returns immediately if none of the
clocks have active timers.
However if at least one of them does, we call qemu_next_alarm_deadline
(that potentially can return INT64_MAX) and then rearm.

So for example if the clock that has active timers is disabled (I don't
if it is possible to get in this state), qemu_next_alarm_deadline would
return INT64_MAX.
I think we should make the appended change in order to make the code
more reliable.

Regarding setitimer returning -EINVAL, we could check for out of bound
parameters in unix_rearm_timer, but we need to know exactly what the
upper limit is. I tried to look for the Darwin manpage of setitimer, but
that information is missing.
Could you please run a couple of tests to find out what the actual limit
is?

---

diff --git a/qemu-timer.c b/qemu-timer.c
index de98977..81ff824 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -112,14 +112,10 @@ static int64_t qemu_next_alarm_deadline(void)
 
 static void qemu_rearm_alarm_timer(struct qemu_alarm_timer *t)
 {
-int64_t nearest_delta_ns;
-if (!rt_clock-active_timers 
-!vm_clock-active_timers 
-!host_clock-active_timers) {
-return;
+int64_t nearest_delta_ns = qemu_next_alarm_deadline();
+if (nearest_delta_ns  INT64_MAX) {
+t-rearm(t, nearest_delta_ns);
 }
-nearest_delta_ns = qemu_next_alarm_deadline();
-t-rearm(t, nearest_delta_ns);
 }
 
 /* TODO: MIN_TIMER_REARM_NS should be optimized */

Re: [Qemu-devel] qtest: setitimer() failures on Darwin and illumos

2012-05-29 Thread Paolo Bonzini
Il 29/05/2012 14:01, Stefano Stabellini ha scritto:
 On Mon, 28 May 2012, Paolo Bonzini wrote:
 Il 28/05/2012 21:40, Andreas Färber ha scritto:
 I'm seeing qemu-timer.c:unix_rearm_timer()'s setitimer() abort with
 EINVAL during `make check` on both platforms. The value of
 nearest_delta_ns appears to be INT64_MAX. Is this expected? Is it
 possible that this value is too large for it_value on some platforms?
 Apple's man page mentions that as possible reason for EINVAL but doesn't
 describe how to obtain such an upper value, nor of course where in the
 QEMU code base we would need to make adaptions. ;)

 Any suggestions?

 You shouldn't call the rearm function at all if you get INT64_MAX.  This
 applies to all timers.
 
 Yep. In fact qemu_rearm_alarm_timer returns immediately if none of the
 clocks have active timers.
 However if at least one of them does, we call qemu_next_alarm_deadline
 (that potentially can return INT64_MAX) and then rearm.
 
 So for example if the clock that has active timers is disabled (I don't
 if it is possible to get in this state), qemu_next_alarm_deadline would
 return INT64_MAX.
 I think we should make the appended change in order to make the code
 more reliable.

Yes, that makes sense, can you submit it with SoB?

Paolo



Re: [Qemu-devel] qtest: setitimer() failures on Darwin and illumos

2012-05-29 Thread Andreas Färber
Am 28.05.2012 22:15, schrieb Paolo Bonzini:
 Il 28/05/2012 21:40, Andreas Färber ha scritto:
 I'm seeing qemu-timer.c:unix_rearm_timer()'s setitimer() abort with
 EINVAL during `make check` on both platforms. The value of
 nearest_delta_ns appears to be INT64_MAX. Is this expected? Is it
 possible that this value is too large for it_value on some platforms?
 Apple's man page mentions that as possible reason for EINVAL but doesn't
 describe how to obtain such an upper value, nor of course where in the
 QEMU code base we would need to make adaptions. ;)

 Any suggestions?
 
 You shouldn't call the rearm function at all if you get INT64_MAX.  This
 applies to all timers.

I had tried doing if (... == INT64_MAX) return; in unix_rearm_timer()
this morning. That avoided the abort, but the process ran at close to
100% CPU and seemed to hang make check. Don't know if that's related to
timers though.

Not a 1.1 release blocker anyway.

It's unfortunate though that qtest has such bad error handling with lots
of assertions - the previous accept() issue on illumos left zombie child
processes behind and on my machine I've seen intermittent hangs of
tests. I can certainly kill processes on my local machine if I notice,
but for our RPM builds I'm avoiding make check for now...

Andreas



Re: [Qemu-devel] qtest: setitimer() failures on Darwin and illumos

2012-05-28 Thread Paolo Bonzini
Il 28/05/2012 21:40, Andreas Färber ha scritto:
 I'm seeing qemu-timer.c:unix_rearm_timer()'s setitimer() abort with
 EINVAL during `make check` on both platforms. The value of
 nearest_delta_ns appears to be INT64_MAX. Is this expected? Is it
 possible that this value is too large for it_value on some platforms?
 Apple's man page mentions that as possible reason for EINVAL but doesn't
 describe how to obtain such an upper value, nor of course where in the
 QEMU code base we would need to make adaptions. ;)
 
 Any suggestions?

You shouldn't call the rearm function at all if you get INT64_MAX.  This
applies to all timers.

We probably never had the problem because until recently the delta was
capped to INT32_MAX instead.

Paolo