[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCHv2] [RFC 1/7] aio / timers: Remove alarm timers

From: Alex Bligh
Subject: Re: [Qemu-devel] [PATCHv2] [RFC 1/7] aio / timers: Remove alarm timers
Date: Thu, 25 Jul 2013 10:37:06 +0100


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.


@@ -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

     /* 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?
+         */
         if (use_icount) {

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;
             case QEMU_OPTION_clock:
-                configure_alarms(optarg);
+                /* Once upon a time we did:
+                 *   configure_alarms(optarg);
+                 * here. This is stubbed out for compatibility.
+                 */

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

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


Alex Bligh

reply via email to

[Prev in Thread] Current Thread [Next in Thread]