Stefan,

This should be one of the last patches so qemu.git remains bisectable.
Only remove the alarm timer once the event loops are already using the
timeout argument.

OK

@@ -245,11 +82,7 @@ static QEMUClock *qemu_new_clock(int type)

 void qemu_clock_enable(QEMUClock *clock, bool enabled)
 {
-    bool old = clock->enabled;
     clock->enabled = enabled;
-    if (enabled && !old) {
-        qemu_rearm_alarm_timer(alarm_timer);
-    }

If this function is supposed to work when called from another thread
(e.g. vcpu thread), then you need to call qemu_notify_event().  For
AioContext clocks that should be aio_notify() with the relevant
AioContext, but we don't need that yet.

Each AioContext knows which clock it has but each clock doesn't know if
it's part of an AioContext. I suggest this is infrequent enough that always
using qemu_notify_event() would be OK. That should interrupt any poll.

 int64_t qemu_clock_has_timers(QEMUClock *clock)
@@ -340,10 +173,9 @@ void qemu_mod_timer_ns(QEMUTimer *ts, int64_t
expire_time)

     /* Rearm if necessary  */
     if (pt == &ts->clock->active_timers) {
-        if (!alarm_timer->pending) {
-            qemu_rearm_alarm_timer(alarm_timer);
-        }
-        /* Interrupt execution to force deadline recalculation.  */
+        /* Interrupt execution to force deadline recalculation.
+         * FIXME: Do we need to do this now?
+         */
         qemu_clock_warp(ts->clock);
         if (use_icount) {
             qemu_notify_event();

Same here.

I think I just need to delete the FIXME comment.

diff --git a/vl.c b/vl.c
index 25b8f2f..612c609 100644
--- a/vl.c
+++ b/vl.c
@@ -3714,7 +3714,10 @@ int main(int argc, char **argv, char **envp)
                 old_param = 1;
                 break;
             case QEMU_OPTION_clock:
-                configure_alarms(optarg);
+                /* Once upon a time we did:
+                 *   configure_alarms(optarg);
+                 * here. This is stubbed out for compatibility.
+                 */
                 break;

This could be made clearer to say outright that the options don't exist
anymore:

/* Clock options no longer exist.  Keep this option for backward
 * compatibility.
 */

OK

--
Alex Bligh

Reply via email to