qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/4] qemu-timer: introduce usable pointer-free API
@ 2015-01-08 10:03 Paolo Bonzini
  2015-01-08 10:03 ` [Qemu-devel] [PATCH 1/4] qemu-timer: rename timer_init to timer_init_tl Paolo Bonzini
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Paolo Bonzini @ 2015-01-08 10:03 UTC (permalink / raw)
  To: qemu-devel

The current pointer free API for timers is very low level.  Introduce
a new API that matches timer_new_ns/us/ms and also a new API timer_deinit
that can be used instead of timer_free.

Finally, mechanically change timer macro names in vmstate, to make it
clear that they accept a pointer.

This prepares for conversion of timers from QEMUTimer * to QEMUTimer.

Paolo

Paolo Bonzini (4):
  qemu-timer: rename timer_init to timer_init_tl
  qemu-timer: add timer_init and timer_init_ns/us/ms
  qemu-timer: introduce timer_deinit
  vmstate: accept QEMUTimer in VMSTATE_TIMER*, add VMSTATE_TIMER_PTR*

 hw/acpi/ich9.c              |  2 +-
 hw/acpi/piix4.c             |  2 +-
 hw/arm/stellaris.c          |  2 +-
 hw/block/fdc.c              |  2 +-
 hw/char/cadence_uart.c      |  2 +-
 hw/char/serial.c            |  4 +-
 hw/core/ptimer.c            |  2 +-
 hw/dma/pl330.c              |  2 +-
 hw/input/lm832x.c           |  2 +-
 hw/intc/armv7m_nvic.c       |  2 +-
 hw/isa/vt82c686.c           |  2 +-
 hw/misc/macio/cuda.c        |  2 +-
 hw/net/pcnet.c              |  2 +-
 hw/sd/sdhci.c               |  4 +-
 hw/timer/a9gtimer.c         |  2 +-
 hw/timer/arm_mptimer.c      |  2 +-
 hw/timer/hpet.c             |  2 +-
 hw/timer/mc146818rtc.c      |  4 +-
 hw/usb/hcd-ehci.c           |  2 +-
 hw/usb/hcd-ohci.c           |  2 +-
 hw/usb/hcd-uhci.c           |  2 +-
 hw/usb/hcd-xhci.c           |  2 +-
 hw/usb/redirect.c           |  2 +-
 hw/watchdog/wdt_i6300esb.c  |  2 +-
 hw/watchdog/wdt_ib700.c     |  2 +-
 include/block/aio.h         |  2 +-
 include/migration/vmstate.h | 18 +++++++--
 include/qemu/timer.h        | 94 ++++++++++++++++++++++++++++++++++++++++++---
 qemu-timer.c                | 20 +++++++---
 target-arm/machine.c        |  4 +-
 30 files changed, 149 insertions(+), 45 deletions(-)

-- 
2.1.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH 1/4] qemu-timer: rename timer_init to timer_init_tl
  2015-01-08 10:03 [Qemu-devel] [PATCH 0/4] qemu-timer: introduce usable pointer-free API Paolo Bonzini
@ 2015-01-08 10:03 ` Paolo Bonzini
  2015-01-08 10:03 ` [Qemu-devel] [PATCH 2/4] qemu-timer: add timer_init and timer_init_ns/us/ms Paolo Bonzini
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2015-01-08 10:03 UTC (permalink / raw)
  To: qemu-devel

timer_init is not called that often.  Free the name for an equivalent
of timer_new.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/block/aio.h  |  2 +-
 include/qemu/timer.h | 10 +++++-----
 qemu-timer.c         |  6 +++---
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/block/aio.h b/include/block/aio.h
index 6bf0e04..7d1e26b 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -314,7 +314,7 @@ static inline void aio_timer_init(AioContext *ctx,
                                   int scale,
                                   QEMUTimerCB *cb, void *opaque)
 {
-    timer_init(ts, ctx->tlg.tl[type], scale, cb, opaque);
+    timer_init_tl(ts, ctx->tlg.tl[type], scale, cb, opaque);
 }
 
 /**
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index d9df094..0666920 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -410,7 +410,7 @@ int64_t timerlistgroup_deadline_ns(QEMUTimerListGroup *tlg);
  */
 
 /**
- * timer_init:
+ * timer_init_tl:
  * @ts: the timer to be initialised
  * @timer_list: the timer list to attach the timer to
  * @scale: the scale value for the timer
@@ -423,9 +423,9 @@ int64_t timerlistgroup_deadline_ns(QEMUTimerListGroup *tlg);
  * You need not call an explicit deinit call. Simply make
  * sure it is not on a list with timer_del.
  */
-void timer_init(QEMUTimer *ts,
-                QEMUTimerList *timer_list, int scale,
-                QEMUTimerCB *cb, void *opaque);
+void timer_init_tl(QEMUTimer *ts,
+                   QEMUTimerList *timer_list, int scale,
+                   QEMUTimerCB *cb, void *opaque);
 
 /**
  * timer_new_tl:
@@ -448,7 +448,7 @@ static inline QEMUTimer *timer_new_tl(QEMUTimerList *timer_list,
                                       void *opaque)
 {
     QEMUTimer *ts = g_malloc0(sizeof(QEMUTimer));
-    timer_init(ts, timer_list, scale, cb, opaque);
+    timer_init_tl(ts, timer_list, scale, cb, opaque);
     return ts;
 }
 
diff --git a/qemu-timer.c b/qemu-timer.c
index cb7d988..98d9d1b 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -331,9 +331,9 @@ int qemu_poll_ns(GPollFD *fds, guint nfds, int64_t timeout)
 }
 
 
-void timer_init(QEMUTimer *ts,
-                QEMUTimerList *timer_list, int scale,
-                QEMUTimerCB *cb, void *opaque)
+void timer_init_tl(QEMUTimer *ts,
+                   QEMUTimerList *timer_list, int scale,
+                   QEMUTimerCB *cb, void *opaque)
 {
     ts->timer_list = timer_list;
     ts->cb = cb;
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH 2/4] qemu-timer: add timer_init and timer_init_ns/us/ms
  2015-01-08 10:03 [Qemu-devel] [PATCH 0/4] qemu-timer: introduce usable pointer-free API Paolo Bonzini
  2015-01-08 10:03 ` [Qemu-devel] [PATCH 1/4] qemu-timer: rename timer_init to timer_init_tl Paolo Bonzini
@ 2015-01-08 10:03 ` Paolo Bonzini
  2015-01-09  2:19   ` Fam Zheng
  2015-01-08 10:03 ` [Qemu-devel] [PATCH 3/4] qemu-timer: introduce timer_deinit Paolo Bonzini
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2015-01-08 10:03 UTC (permalink / raw)
  To: qemu-devel

These functions for the main loop TimerListGroup will replace
timer_new and timer_new_ns/us/ms.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/timer.h | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 0666920..9f44233 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -428,6 +428,79 @@ void timer_init_tl(QEMUTimer *ts,
                    QEMUTimerCB *cb, void *opaque);
 
 /**
+ * timer_init:
+ * @clock: the clock to associate with the timer
+ * @scale: the scale value for the timer
+ * @cb: the callback to call when the timer expires
+ * @opaque: the opaque pointer to pass to the callback
+ *
+ * Create a new timer with the given scale on the default timer list
+ * associated with the clock.
+ *
+ * You need not call an explicit deinit call. Simply make
+ * sure it is not on a list with timer_del.
+ */
+static inline void timer_init(QEMUTimer *ts, QEMUClockType type, int scale,
+                              QEMUTimerCB *cb, void *opaque)
+{
+    timer_init_tl(ts, main_loop_tlg.tl[type], scale, cb, opaque);
+}
+
+/**
+ * timer_init_ns:
+ * @clock: the clock to associate with the timer
+ * @cb: the callback to call when the timer expires
+ * @opaque: the opaque pointer to pass to the callback
+ *
+ * Create a new timer with nanosecond scale on the default timer list
+ * associated with the clock.
+ *
+ * You need not call an explicit deinit call. Simply make
+ * sure it is not on a list with timer_del.
+ */
+static inline void timer_init_ns(QEMUTimer *ts, QEMUClockType type,
+                                 QEMUTimerCB *cb, void *opaque)
+{
+    timer_init(ts, type, SCALE_NS, cb, opaque);
+}
+
+/**
+ * timer_init_us:
+ * @clock: the clock to associate with the timer
+ * @cb: the callback to call when the timer expires
+ * @opaque: the opaque pointer to pass to the callback
+ *
+ * Create a new timer with microsecond scale on the default timer list
+ * associated with the clock.
+ *
+ * You need not call an explicit deinit call. Simply make
+ * sure it is not on a list with timer_del.
+ */
+static inline void timer_init_us(QEMUTimer *ts, QEMUClockType type,
+                                 QEMUTimerCB *cb, void *opaque)
+{
+    timer_init(ts, type, SCALE_US, cb, opaque);
+}
+
+/**
+ * timer_init_ms:
+ * @clock: the clock to associate with the timer
+ * @cb: the callback to call when the timer expires
+ * @opaque: the opaque pointer to pass to the callback
+ *
+ * Create a new timer with millisecond scale on the default timer list
+ * associated with the clock.
+ *
+ * You need not call an explicit deinit call. Simply make
+ * sure it is not on a list with timer_del.
+ */
+static inline void timer_init_ms(QEMUTimer *ts, QEMUClockType type,
+                                 QEMUTimerCB *cb, void *opaque)
+{
+    timer_init(ts, type, SCALE_MS, cb, opaque);
+}
+
+/**
  * timer_new_tl:
  * @timer_list: the timer list to attach the timer to
  * @scale: the scale value for the timer
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH 3/4] qemu-timer: introduce timer_deinit
  2015-01-08 10:03 [Qemu-devel] [PATCH 0/4] qemu-timer: introduce usable pointer-free API Paolo Bonzini
  2015-01-08 10:03 ` [Qemu-devel] [PATCH 1/4] qemu-timer: rename timer_init to timer_init_tl Paolo Bonzini
  2015-01-08 10:03 ` [Qemu-devel] [PATCH 2/4] qemu-timer: add timer_init and timer_init_ns/us/ms Paolo Bonzini
@ 2015-01-08 10:03 ` Paolo Bonzini
  2015-01-08 10:03 ` [Qemu-devel] [PATCH 4/4] vmstate: accept QEMUTimer in VMSTATE_TIMER*, add VMSTATE_TIMER_PTR* Paolo Bonzini
  2015-01-09  2:10 ` [Qemu-devel] [PATCH 0/4] qemu-timer: introduce usable pointer-free API Fam Zheng
  4 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2015-01-08 10:03 UTC (permalink / raw)
  To: qemu-devel

In some cases, a timer was set to NULL so that we could check if it is
initialized.  Use the timer_list field instead, and add a timer_deinit
function that NULLs it.

It then makes sense that timer_del be a no-op (instead of a crasher) on
such a de-initialized timer.  It avoids the need to poke at the timerlist
field to check if the timers are initialized.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/timer.h | 11 +++++++++++
 qemu-timer.c         | 14 +++++++++++---
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 9f44233..5d5802f 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -595,6 +595,17 @@ static inline QEMUTimer *timer_new_ms(QEMUClockType type, QEMUTimerCB *cb,
 }
 
 /**
+ * timer_deinit:
+ * @ts: the timer to be de-initialised
+ *
+ * Deassociate the timer from any timerlist.  You should
+ * call timer_del before.  After this call, any further
+ * timer_del call cannot cause dangling pointer accesses
+ * even if the previously used timerlist is freed.
+ */
+void timer_deinit(QEMUTimer *ts);
+
+/**
  * timer_free:
  * @ts: the timer
  *
diff --git a/qemu-timer.c b/qemu-timer.c
index 98d9d1b..464396f 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -342,6 +342,12 @@ void timer_init_tl(QEMUTimer *ts,
     ts->expire_time = -1;
 }
 
+void timer_deinit(QEMUTimer *ts)
+{
+    assert(ts->expire_time == -1);
+    ts->timer_list = NULL;
+}
+
 void timer_free(QEMUTimer *ts)
 {
     g_free(ts);
@@ -398,9 +404,11 @@ void timer_del(QEMUTimer *ts)
 {
     QEMUTimerList *timer_list = ts->timer_list;
 
-    qemu_mutex_lock(&timer_list->active_timers_lock);
-    timer_del_locked(timer_list, ts);
-    qemu_mutex_unlock(&timer_list->active_timers_lock);
+    if (timer_list) {
+        qemu_mutex_lock(&timer_list->active_timers_lock);
+        timer_del_locked(timer_list, ts);
+        qemu_mutex_unlock(&timer_list->active_timers_lock);
+    }
 }
 
 /* modify the current timer so that it will be fired when current_time
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH 4/4] vmstate: accept QEMUTimer in VMSTATE_TIMER*, add VMSTATE_TIMER_PTR*
  2015-01-08 10:03 [Qemu-devel] [PATCH 0/4] qemu-timer: introduce usable pointer-free API Paolo Bonzini
                   ` (2 preceding siblings ...)
  2015-01-08 10:03 ` [Qemu-devel] [PATCH 3/4] qemu-timer: introduce timer_deinit Paolo Bonzini
@ 2015-01-08 10:03 ` Paolo Bonzini
  2015-01-15  9:04   ` Amit Shah
  2015-01-09  2:10 ` [Qemu-devel] [PATCH 0/4] qemu-timer: introduce usable pointer-free API Fam Zheng
  4 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2015-01-08 10:03 UTC (permalink / raw)
  To: qemu-devel

Old users of VMSTATE_TIMER* are mechanically changed to VMSTATE_TIMER_PTR
variants.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/acpi/ich9.c              |  2 +-
 hw/acpi/piix4.c             |  2 +-
 hw/arm/stellaris.c          |  2 +-
 hw/block/fdc.c              |  2 +-
 hw/char/cadence_uart.c      |  2 +-
 hw/char/serial.c            |  4 ++--
 hw/core/ptimer.c            |  2 +-
 hw/dma/pl330.c              |  2 +-
 hw/input/lm832x.c           |  2 +-
 hw/intc/armv7m_nvic.c       |  2 +-
 hw/isa/vt82c686.c           |  2 +-
 hw/misc/macio/cuda.c        |  2 +-
 hw/net/pcnet.c              |  2 +-
 hw/sd/sdhci.c               |  4 ++--
 hw/timer/a9gtimer.c         |  2 +-
 hw/timer/arm_mptimer.c      |  2 +-
 hw/timer/hpet.c             |  2 +-
 hw/timer/mc146818rtc.c      |  4 ++--
 hw/usb/hcd-ehci.c           |  2 +-
 hw/usb/hcd-ohci.c           |  2 +-
 hw/usb/hcd-uhci.c           |  2 +-
 hw/usb/hcd-xhci.c           |  2 +-
 hw/usb/redirect.c           |  2 +-
 hw/watchdog/wdt_i6300esb.c  |  2 +-
 hw/watchdog/wdt_ib700.c     |  2 +-
 include/migration/vmstate.h | 18 +++++++++++++++---
 target-arm/machine.c        |  4 ++--
 27 files changed, 45 insertions(+), 33 deletions(-)

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index ea991a3..43869d7 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -166,7 +166,7 @@ const VMStateDescription vmstate_ich9_pm = {
         VMSTATE_UINT16(acpi_regs.pm1.evt.sts, ICH9LPCPMRegs),
         VMSTATE_UINT16(acpi_regs.pm1.evt.en, ICH9LPCPMRegs),
         VMSTATE_UINT16(acpi_regs.pm1.cnt.cnt, ICH9LPCPMRegs),
-        VMSTATE_TIMER(acpi_regs.tmr.timer, ICH9LPCPMRegs),
+        VMSTATE_TIMER_PTR(acpi_regs.tmr.timer, ICH9LPCPMRegs),
         VMSTATE_INT64(acpi_regs.tmr.overflow_time, ICH9LPCPMRegs),
         VMSTATE_GPE_ARRAY(acpi_regs.gpe.sts, ICH9LPCPMRegs),
         VMSTATE_GPE_ARRAY(acpi_regs.gpe.en, ICH9LPCPMRegs),
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 481a16c..184e7e4 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -285,7 +285,7 @@ static const VMStateDescription vmstate_acpi = {
         VMSTATE_UINT16(ar.pm1.evt.en, PIIX4PMState),
         VMSTATE_UINT16(ar.pm1.cnt.cnt, PIIX4PMState),
         VMSTATE_STRUCT(apm, PIIX4PMState, 0, vmstate_apm, APMState),
-        VMSTATE_TIMER(ar.tmr.timer, PIIX4PMState),
+        VMSTATE_TIMER_PTR(ar.tmr.timer, PIIX4PMState),
         VMSTATE_INT64(ar.tmr.overflow_time, PIIX4PMState),
         VMSTATE_STRUCT(ar.gpe, PIIX4PMState, 2, vmstate_gpe, ACPIGPE),
         VMSTATE_STRUCT_TEST(
diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index 64bd4b4..ccc3b18 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -306,7 +306,7 @@ static const VMStateDescription vmstate_stellaris_gptm = {
         VMSTATE_UINT32_ARRAY(match_prescale, gptm_state, 2),
         VMSTATE_UINT32(rtc, gptm_state),
         VMSTATE_INT64_ARRAY(tick, gptm_state, 2),
-        VMSTATE_TIMER_ARRAY(timer, gptm_state, 2),
+        VMSTATE_TIMER_PTR_ARRAY(timer, gptm_state, 2),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 739a03e..2bf87c9 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -791,7 +791,7 @@ static const VMStateDescription vmstate_fdc_result_timer = {
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
-        VMSTATE_TIMER(result_timer, FDCtrl),
+        VMSTATE_TIMER_PTR(result_timer, FDCtrl),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
index a5736cb..7044b35 100644
--- a/hw/char/cadence_uart.c
+++ b/hw/char/cadence_uart.c
@@ -520,7 +520,7 @@ static const VMStateDescription vmstate_cadence_uart = {
         VMSTATE_UINT32(rx_count, UartState),
         VMSTATE_UINT32(tx_count, UartState),
         VMSTATE_UINT32(rx_wpos, UartState),
-        VMSTATE_TIMER(fifo_trigger_handle, UartState),
+        VMSTATE_TIMER_PTR(fifo_trigger_handle, UartState),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/hw/char/serial.c b/hw/char/serial.c
index 6d522ff..c5c0546 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -721,7 +721,7 @@ const VMStateDescription vmstate_serial_fifo_timeout_timer = {
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
-        VMSTATE_TIMER(fifo_timeout_timer, SerialState),
+        VMSTATE_TIMER_PTR(fifo_timeout_timer, SerialState),
         VMSTATE_END_OF_LIST()
     }
 };
@@ -754,7 +754,7 @@ const VMStateDescription vmstate_serial_poll = {
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
         VMSTATE_INT32(poll_msl, SerialState),
-        VMSTATE_TIMER(modem_status_poll, SerialState),
+        VMSTATE_TIMER_PTR(modem_status_poll, SerialState),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index 466e543..2abad1f 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -214,7 +214,7 @@ const VMStateDescription vmstate_ptimer = {
         VMSTATE_INT64(period, ptimer_state),
         VMSTATE_INT64(last_event, ptimer_state),
         VMSTATE_INT64(next_event, ptimer_state),
-        VMSTATE_TIMER(timer, ptimer_state),
+        VMSTATE_TIMER_PTR(timer, ptimer_state),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/hw/dma/pl330.c b/hw/dma/pl330.c
index 6b6eaae..16cf77e 100644
--- a/hw/dma/pl330.c
+++ b/hw/dma/pl330.c
@@ -286,7 +286,7 @@ static const VMStateDescription vmstate_pl330 = {
                        PL330Queue),
         VMSTATE_STRUCT(write_queue, PL330State, 0, vmstate_pl330_queue,
                        PL330Queue),
-        VMSTATE_TIMER(timer, PL330State),
+        VMSTATE_TIMER_PTR(timer, PL330State),
         VMSTATE_UINT32(inten, PL330State),
         VMSTATE_UINT32(int_status, PL330State),
         VMSTATE_UINT32(ev_status, PL330State),
diff --git a/hw/input/lm832x.c b/hw/input/lm832x.c
index 9eb68e8..530a6e0 100644
--- a/hw/input/lm832x.c
+++ b/hw/input/lm832x.c
@@ -455,7 +455,7 @@ static const VMStateDescription vmstate_lm_kbd = {
         VMSTATE_UINT16_ARRAY(pwm.file, LM823KbdState, 256),
         VMSTATE_UINT8(pwm.faddr, LM823KbdState),
         VMSTATE_BUFFER(pwm.addr, LM823KbdState),
-        VMSTATE_TIMER_ARRAY(pwm.tm, LM823KbdState, 3),
+        VMSTATE_TIMER_PTR_ARRAY(pwm.tm, LM823KbdState, 3),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index d0543d4..6ff6c7f 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -450,7 +450,7 @@ static const VMStateDescription vmstate_nvic = {
         VMSTATE_UINT32(systick.control, nvic_state),
         VMSTATE_UINT32(systick.reload, nvic_state),
         VMSTATE_INT64(systick.tick, nvic_state),
-        VMSTATE_TIMER(systick.timer, nvic_state),
+        VMSTATE_TIMER_PTR(systick.timer, nvic_state),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 223b947..a3ce0ee 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -234,7 +234,7 @@ static const VMStateDescription vmstate_acpi = {
         VMSTATE_UINT16(ar.pm1.evt.en, VT686PMState),
         VMSTATE_UINT16(ar.pm1.cnt.cnt, VT686PMState),
         VMSTATE_STRUCT(apm, VT686PMState, 0, vmstate_apm, APMState),
-        VMSTATE_TIMER(ar.tmr.timer, VT686PMState),
+        VMSTATE_TIMER_PTR(ar.tmr.timer, VT686PMState),
         VMSTATE_INT64(ar.tmr.overflow_time, VT686PMState),
         VMSTATE_END_OF_LIST()
     }
diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index b4273aa..47d9771 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -631,7 +631,7 @@ static const VMStateDescription vmstate_cuda_timer = {
         VMSTATE_UINT16(counter_value, CUDATimer),
         VMSTATE_INT64(load_time, CUDATimer),
         VMSTATE_INT64(next_irq_time, CUDATimer),
-        VMSTATE_TIMER_TEST(timer, CUDATimer, cuda_timer_exist),
+        VMSTATE_TIMER_PTR_TEST(timer, CUDATimer, cuda_timer_exist),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index f409b92..cbaaadc 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -1719,7 +1719,7 @@ const VMStateDescription vmstate_pcnet = {
         VMSTATE_BUFFER(buffer, PCNetState),
         VMSTATE_UNUSED_TEST(is_version_2, 4),
         VMSTATE_INT32(tx_busy, PCNetState),
-        VMSTATE_TIMER(poll_timer, PCNetState),
+        VMSTATE_TIMER_PTR(poll_timer, PCNetState),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 15064d3..10e5355 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1205,8 +1205,8 @@ const VMStateDescription sdhci_vmstate = {
         VMSTATE_UINT64(admasysaddr, SDHCIState),
         VMSTATE_UINT8(stopped_state, SDHCIState),
         VMSTATE_VBUFFER_UINT32(fifo_buffer, SDHCIState, 1, NULL, 0, buf_maxsz),
-        VMSTATE_TIMER(insert_timer, SDHCIState),
-        VMSTATE_TIMER(transfer_timer, SDHCIState),
+        VMSTATE_TIMER_PTR(insert_timer, SDHCIState),
+        VMSTATE_TIMER_PTR(transfer_timer, SDHCIState),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/hw/timer/a9gtimer.c b/hw/timer/a9gtimer.c
index a0656d5..435142a 100644
--- a/hw/timer/a9gtimer.c
+++ b/hw/timer/a9gtimer.c
@@ -328,7 +328,7 @@ static const VMStateDescription vmstate_a9_gtimer = {
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
-        VMSTATE_TIMER(timer, A9GTimerState),
+        VMSTATE_TIMER_PTR(timer, A9GTimerState),
         VMSTATE_UINT64(counter, A9GTimerState),
         VMSTATE_UINT64(ref_counter, A9GTimerState),
         VMSTATE_UINT64(cpu_ref_time, A9GTimerState),
diff --git a/hw/timer/arm_mptimer.c b/hw/timer/arm_mptimer.c
index 35a0a23..8b93b3c 100644
--- a/hw/timer/arm_mptimer.c
+++ b/hw/timer/arm_mptimer.c
@@ -246,7 +246,7 @@ static const VMStateDescription vmstate_timerblock = {
         VMSTATE_UINT32(control, TimerBlock),
         VMSTATE_UINT32(status, TimerBlock),
         VMSTATE_INT64(tick, TimerBlock),
-        VMSTATE_TIMER(timer, TimerBlock),
+        VMSTATE_TIMER_PTR(timer, TimerBlock),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index d8bc231..78d86be 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -299,7 +299,7 @@ static const VMStateDescription vmstate_hpet_timer = {
         VMSTATE_UINT64(fsb, HPETTimer),
         VMSTATE_UINT64(period, HPETTimer),
         VMSTATE_UINT8(wrap_flag, HPETTimer),
-        VMSTATE_TIMER(qemu_timer, HPETTimer),
+        VMSTATE_TIMER_PTR(qemu_timer, HPETTimer),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index f18d128..5a107fa 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -758,7 +758,7 @@ static const VMStateDescription vmstate_rtc = {
         VMSTATE_BUFFER(cmos_data, RTCState),
         VMSTATE_UINT8(cmos_index, RTCState),
         VMSTATE_UNUSED(7*4),
-        VMSTATE_TIMER(periodic_timer, RTCState),
+        VMSTATE_TIMER_PTR(periodic_timer, RTCState),
         VMSTATE_INT64(next_periodic_time, RTCState),
         VMSTATE_UNUSED(3*8),
         VMSTATE_UINT32_V(irq_coalesced, RTCState, 2),
@@ -766,7 +766,7 @@ static const VMStateDescription vmstate_rtc = {
         VMSTATE_UINT64_V(base_rtc, RTCState, 3),
         VMSTATE_UINT64_V(last_update, RTCState, 3),
         VMSTATE_INT64_V(offset, RTCState, 3),
-        VMSTATE_TIMER_V(update_timer, RTCState, 3),
+        VMSTATE_TIMER_PTR_V(update_timer, RTCState, 3),
         VMSTATE_UINT64_V(next_alarm_time, RTCState, 3),
         VMSTATE_END_OF_LIST()
     },
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 1cc0fc1..ccf54b6 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2437,7 +2437,7 @@ const VMStateDescription vmstate_ehci = {
         VMSTATE_UINT32(portsc[4], EHCIState),
         VMSTATE_UINT32(portsc[5], EHCIState),
         /* frame timer */
-        VMSTATE_TIMER(frame_timer, EHCIState),
+        VMSTATE_TIMER_PTR(frame_timer, EHCIState),
         VMSTATE_UINT64(last_run_ns, EHCIState),
         VMSTATE_UINT32(async_stepdown, EHCIState),
         /* schedule state */
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 1977093..e9868b8 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -2012,7 +2012,7 @@ static const VMStateDescription vmstate_ohci_eof_timer = {
     .minimum_version_id = 1,
     .pre_load = ohci_eof_timer_pre_load,
     .fields = (VMStateField[]) {
-        VMSTATE_TIMER(eof_timer, OHCIState),
+        VMSTATE_TIMER_PTR(eof_timer, OHCIState),
         VMSTATE_END_OF_LIST()
     },
 };
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 4a4215d..f903de7 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -419,7 +419,7 @@ static const VMStateDescription vmstate_uhci = {
         VMSTATE_UINT32(fl_base_addr, UHCIState),
         VMSTATE_UINT8(sof_timing, UHCIState),
         VMSTATE_UINT8(status2, UHCIState),
-        VMSTATE_TIMER(frame_timer, UHCIState),
+        VMSTATE_TIMER_PTR(frame_timer, UHCIState),
         VMSTATE_INT64_V(expire_time, UHCIState, 2),
         VMSTATE_UINT32_V(pending_int_mask, UHCIState, 3),
         VMSTATE_END_OF_LIST()
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 9a942cf..776699b 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3855,7 +3855,7 @@ static const VMStateDescription vmstate_xhci = {
 
         /* Runtime Registers & state */
         VMSTATE_INT64(mfindex_start,  XHCIState),
-        VMSTATE_TIMER(mfwrap_timer,   XHCIState),
+        VMSTATE_TIMER_PTR(mfwrap_timer,   XHCIState),
         VMSTATE_STRUCT(cmd_ring, XHCIState, 1, vmstate_xhci_ring, XHCIRing),
 
         VMSTATE_END_OF_LIST()
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 9fbd59e..962d3f5 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -2438,7 +2438,7 @@ static const VMStateDescription usbredir_vmstate = {
     .post_load = usbredir_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_USB_DEVICE(dev, USBRedirDevice),
-        VMSTATE_TIMER(attach_timer, USBRedirDevice),
+        VMSTATE_TIMER_PTR(attach_timer, USBRedirDevice),
         {
             .name         = "parser",
             .version_id   = 0,
diff --git a/hw/watchdog/wdt_i6300esb.c b/hw/watchdog/wdt_i6300esb.c
index 687c8b1..33dd6d4 100644
--- a/hw/watchdog/wdt_i6300esb.c
+++ b/hw/watchdog/wdt_i6300esb.c
@@ -398,7 +398,7 @@ static const VMStateDescription vmstate_i6300esb = {
         VMSTATE_INT32(free_run, I6300State),
         VMSTATE_INT32(locked, I6300State),
         VMSTATE_INT32(enabled, I6300State),
-        VMSTATE_TIMER(timer, I6300State),
+        VMSTATE_TIMER_PTR(timer, I6300State),
         VMSTATE_UINT32(timer1_preload, I6300State),
         VMSTATE_UINT32(timer2_preload, I6300State),
         VMSTATE_INT32(stage, I6300State),
diff --git a/hw/watchdog/wdt_ib700.c b/hw/watchdog/wdt_ib700.c
index 8cb9827..0917a71 100644
--- a/hw/watchdog/wdt_ib700.c
+++ b/hw/watchdog/wdt_ib700.c
@@ -93,7 +93,7 @@ static const VMStateDescription vmstate_ib700 = {
     .version_id = 0,
     .minimum_version_id = 0,
     .fields = (VMStateField[]) {
-        VMSTATE_TIMER(timer, IB700State),
+        VMSTATE_TIMER_PTR(timer, IB700State),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index e45fc49..55ba584 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -642,17 +642,29 @@ extern const VMStateInfo vmstate_info_bitmap;
 #define VMSTATE_FLOAT64(_f, _s)                                       \
     VMSTATE_FLOAT64_V(_f, _s, 0)
 
-#define VMSTATE_TIMER_TEST(_f, _s, _test)                             \
+#define VMSTATE_TIMER_PTR_TEST(_f, _s, _test)                             \
     VMSTATE_POINTER_TEST(_f, _s, _test, vmstate_info_timer, QEMUTimer *)
 
-#define VMSTATE_TIMER_V(_f, _s, _v)                                   \
+#define VMSTATE_TIMER_PTR_V(_f, _s, _v)                                   \
     VMSTATE_POINTER(_f, _s, _v, vmstate_info_timer, QEMUTimer *)
 
+#define VMSTATE_TIMER_PTR(_f, _s)                                         \
+    VMSTATE_TIMER_PTR_V(_f, _s, 0)
+
+#define VMSTATE_TIMER_PTR_ARRAY(_f, _s, _n)                              \
+    VMSTATE_ARRAY_OF_POINTER(_f, _s, _n, 0, vmstate_info_timer, QEMUTimer *)
+
+#define VMSTATE_TIMER_TEST(_f, _s, _test)                             \
+    VMSTATE_SINGLE_TEST(_f, _s, _test, 0, vmstate_info_timer, QEMUTimer)
+
+#define VMSTATE_TIMER_V(_f, _s, _v)                                   \
+    VMSTATE_SINGLE(_f, _s, _v, vmstate_info_timer, QEMUTimer)
+
 #define VMSTATE_TIMER(_f, _s)                                         \
     VMSTATE_TIMER_V(_f, _s, 0)
 
 #define VMSTATE_TIMER_ARRAY(_f, _s, _n)                              \
-    VMSTATE_ARRAY_OF_POINTER(_f, _s, _n, 0, vmstate_info_timer, QEMUTimer *)
+    VMSTATE_ARRAY(_f, _s, _n, 0, vmstate_info_timer, QEMUTimer)
 
 #define VMSTATE_BOOL_ARRAY_V(_f, _s, _n, _v)                         \
     VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_bool, bool)
diff --git a/target-arm/machine.c b/target-arm/machine.c
index c29e7a2..9446e5a 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -277,8 +277,8 @@ const VMStateDescription vmstate_arm_cpu = {
         VMSTATE_UINT32(env.exception.syndrome, ARMCPU),
         VMSTATE_UINT32(env.exception.fsr, ARMCPU),
         VMSTATE_UINT64(env.exception.vaddress, ARMCPU),
-        VMSTATE_TIMER(gt_timer[GTIMER_PHYS], ARMCPU),
-        VMSTATE_TIMER(gt_timer[GTIMER_VIRT], ARMCPU),
+        VMSTATE_TIMER_PTR(gt_timer[GTIMER_PHYS], ARMCPU),
+        VMSTATE_TIMER_PTR(gt_timer[GTIMER_VIRT], ARMCPU),
         VMSTATE_BOOL(powered_off, ARMCPU),
         VMSTATE_END_OF_LIST()
     },
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 0/4] qemu-timer: introduce usable pointer-free API
  2015-01-08 10:03 [Qemu-devel] [PATCH 0/4] qemu-timer: introduce usable pointer-free API Paolo Bonzini
                   ` (3 preceding siblings ...)
  2015-01-08 10:03 ` [Qemu-devel] [PATCH 4/4] vmstate: accept QEMUTimer in VMSTATE_TIMER*, add VMSTATE_TIMER_PTR* Paolo Bonzini
@ 2015-01-09  2:10 ` Fam Zheng
  2015-01-09  9:07   ` Paolo Bonzini
  4 siblings, 1 reply; 12+ messages in thread
From: Fam Zheng @ 2015-01-09  2:10 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

On Thu, 01/08 11:03, Paolo Bonzini wrote:
> The current pointer free API for timers is very low level.  Introduce
> a new API that matches timer_new_ns/us/ms and also a new API timer_deinit
> that can be used instead of timer_free.
> 
> Finally, mechanically change timer macro names in vmstate, to make it
> clear that they accept a pointer.
> 
> This prepares for conversion of timers from QEMUTimer * to QEMUTimer.

Novice question: what's the advantage, please?

Fam

> 
> Paolo
> 
> Paolo Bonzini (4):
>   qemu-timer: rename timer_init to timer_init_tl
>   qemu-timer: add timer_init and timer_init_ns/us/ms
>   qemu-timer: introduce timer_deinit
>   vmstate: accept QEMUTimer in VMSTATE_TIMER*, add VMSTATE_TIMER_PTR*
> 
>  hw/acpi/ich9.c              |  2 +-
>  hw/acpi/piix4.c             |  2 +-
>  hw/arm/stellaris.c          |  2 +-
>  hw/block/fdc.c              |  2 +-
>  hw/char/cadence_uart.c      |  2 +-
>  hw/char/serial.c            |  4 +-
>  hw/core/ptimer.c            |  2 +-
>  hw/dma/pl330.c              |  2 +-
>  hw/input/lm832x.c           |  2 +-
>  hw/intc/armv7m_nvic.c       |  2 +-
>  hw/isa/vt82c686.c           |  2 +-
>  hw/misc/macio/cuda.c        |  2 +-
>  hw/net/pcnet.c              |  2 +-
>  hw/sd/sdhci.c               |  4 +-
>  hw/timer/a9gtimer.c         |  2 +-
>  hw/timer/arm_mptimer.c      |  2 +-
>  hw/timer/hpet.c             |  2 +-
>  hw/timer/mc146818rtc.c      |  4 +-
>  hw/usb/hcd-ehci.c           |  2 +-
>  hw/usb/hcd-ohci.c           |  2 +-
>  hw/usb/hcd-uhci.c           |  2 +-
>  hw/usb/hcd-xhci.c           |  2 +-
>  hw/usb/redirect.c           |  2 +-
>  hw/watchdog/wdt_i6300esb.c  |  2 +-
>  hw/watchdog/wdt_ib700.c     |  2 +-
>  include/block/aio.h         |  2 +-
>  include/migration/vmstate.h | 18 +++++++--
>  include/qemu/timer.h        | 94 ++++++++++++++++++++++++++++++++++++++++++---
>  qemu-timer.c                | 20 +++++++---
>  target-arm/machine.c        |  4 +-
>  30 files changed, 149 insertions(+), 45 deletions(-)
> 
> -- 
> 2.1.0
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 2/4] qemu-timer: add timer_init and timer_init_ns/us/ms
  2015-01-08 10:03 ` [Qemu-devel] [PATCH 2/4] qemu-timer: add timer_init and timer_init_ns/us/ms Paolo Bonzini
@ 2015-01-09  2:19   ` Fam Zheng
  2015-01-09 10:39     ` Paolo Bonzini
  0 siblings, 1 reply; 12+ messages in thread
From: Fam Zheng @ 2015-01-09  2:19 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

On Thu, 01/08 11:03, Paolo Bonzini wrote:
> These functions for the main loop TimerListGroup will replace
> timer_new and timer_new_ns/us/ms.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/qemu/timer.h | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
> 
> diff --git a/include/qemu/timer.h b/include/qemu/timer.h
> index 0666920..9f44233 100644
> --- a/include/qemu/timer.h
> +++ b/include/qemu/timer.h
> @@ -428,6 +428,79 @@ void timer_init_tl(QEMUTimer *ts,
>                     QEMUTimerCB *cb, void *opaque);
>  
>  /**
> + * timer_init:
> + * @clock: the clock to associate with the timer

s/@clock/@type/

And similarly below.

> + * @scale: the scale value for the timer
> + * @cb: the callback to call when the timer expires
> + * @opaque: the opaque pointer to pass to the callback
> + *
> + * Create a new timer with the given scale on the default timer list

s/Create a new/Initialize a/ ?

And similarly below.

> + * associated with the clock.
> + *
> + * You need not call an explicit deinit call. Simply make
> + * sure it is not on a list with timer_del.
> + */
> +static inline void timer_init(QEMUTimer *ts, QEMUClockType type, int scale,
> +                              QEMUTimerCB *cb, void *opaque)
> +{
> +    timer_init_tl(ts, main_loop_tlg.tl[type], scale, cb, opaque);
> +}
> +
> +/**
> + * timer_init_ns:
> + * @clock: the clock to associate with the timer
> + * @cb: the callback to call when the timer expires
> + * @opaque: the opaque pointer to pass to the callback
> + *
> + * Create a new timer with nanosecond scale on the default timer list
> + * associated with the clock.
> + *
> + * You need not call an explicit deinit call. Simply make
> + * sure it is not on a list with timer_del.
> + */
> +static inline void timer_init_ns(QEMUTimer *ts, QEMUClockType type,
> +                                 QEMUTimerCB *cb, void *opaque)
> +{
> +    timer_init(ts, type, SCALE_NS, cb, opaque);
> +}
> +
> +/**
> + * timer_init_us:
> + * @clock: the clock to associate with the timer
> + * @cb: the callback to call when the timer expires
> + * @opaque: the opaque pointer to pass to the callback
> + *
> + * Create a new timer with microsecond scale on the default timer list
> + * associated with the clock.
> + *
> + * You need not call an explicit deinit call. Simply make
> + * sure it is not on a list with timer_del.
> + */
> +static inline void timer_init_us(QEMUTimer *ts, QEMUClockType type,
> +                                 QEMUTimerCB *cb, void *opaque)
> +{
> +    timer_init(ts, type, SCALE_US, cb, opaque);
> +}
> +
> +/**
> + * timer_init_ms:
> + * @clock: the clock to associate with the timer
> + * @cb: the callback to call when the timer expires
> + * @opaque: the opaque pointer to pass to the callback
> + *
> + * Create a new timer with millisecond scale on the default timer list
> + * associated with the clock.
> + *
> + * You need not call an explicit deinit call. Simply make
> + * sure it is not on a list with timer_del.
> + */
> +static inline void timer_init_ms(QEMUTimer *ts, QEMUClockType type,
> +                                 QEMUTimerCB *cb, void *opaque)
> +{
> +    timer_init(ts, type, SCALE_MS, cb, opaque);
> +}
> +
> +/**
>   * timer_new_tl:
>   * @timer_list: the timer list to attach the timer to
>   * @scale: the scale value for the timer
> -- 
> 2.1.0
> 
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 0/4] qemu-timer: introduce usable pointer-free API
  2015-01-09  2:10 ` [Qemu-devel] [PATCH 0/4] qemu-timer: introduce usable pointer-free API Fam Zheng
@ 2015-01-09  9:07   ` Paolo Bonzini
  2015-01-10  1:35     ` Fam Zheng
  0 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2015-01-09  9:07 UTC (permalink / raw)
  To: Fam Zheng; +Cc: qemu-devel



On 09/01/2015 03:10, Fam Zheng wrote:
> On Thu, 01/08 11:03, Paolo Bonzini wrote:
>> The current pointer free API for timers is very low level.  Introduce
>> a new API that matches timer_new_ns/us/ms and also a new API timer_deinit
>> that can be used instead of timer_free.
>>
>> Finally, mechanically change timer macro names in vmstate, to make it
>> clear that they accept a pointer.
>>
>> This prepares for conversion of timers from QEMUTimer * to QEMUTimer.
> 
> Novice question: what's the advantage, please?

No need to free them, so no memory leaks.  From a quick look I found two
(hw/net/pcnet.c and hw/char/serial.c).

Also it matches more recent device model APIs like memory regions or
throttling.

Paolo

> Fam
> 
>>
>> Paolo
>>
>> Paolo Bonzini (4):
>>   qemu-timer: rename timer_init to timer_init_tl
>>   qemu-timer: add timer_init and timer_init_ns/us/ms
>>   qemu-timer: introduce timer_deinit
>>   vmstate: accept QEMUTimer in VMSTATE_TIMER*, add VMSTATE_TIMER_PTR*
>>
>>  hw/acpi/ich9.c              |  2 +-
>>  hw/acpi/piix4.c             |  2 +-
>>  hw/arm/stellaris.c          |  2 +-
>>  hw/block/fdc.c              |  2 +-
>>  hw/char/cadence_uart.c      |  2 +-
>>  hw/char/serial.c            |  4 +-
>>  hw/core/ptimer.c            |  2 +-
>>  hw/dma/pl330.c              |  2 +-
>>  hw/input/lm832x.c           |  2 +-
>>  hw/intc/armv7m_nvic.c       |  2 +-
>>  hw/isa/vt82c686.c           |  2 +-
>>  hw/misc/macio/cuda.c        |  2 +-
>>  hw/net/pcnet.c              |  2 +-
>>  hw/sd/sdhci.c               |  4 +-
>>  hw/timer/a9gtimer.c         |  2 +-
>>  hw/timer/arm_mptimer.c      |  2 +-
>>  hw/timer/hpet.c             |  2 +-
>>  hw/timer/mc146818rtc.c      |  4 +-
>>  hw/usb/hcd-ehci.c           |  2 +-
>>  hw/usb/hcd-ohci.c           |  2 +-
>>  hw/usb/hcd-uhci.c           |  2 +-
>>  hw/usb/hcd-xhci.c           |  2 +-
>>  hw/usb/redirect.c           |  2 +-
>>  hw/watchdog/wdt_i6300esb.c  |  2 +-
>>  hw/watchdog/wdt_ib700.c     |  2 +-
>>  include/block/aio.h         |  2 +-
>>  include/migration/vmstate.h | 18 +++++++--
>>  include/qemu/timer.h        | 94 ++++++++++++++++++++++++++++++++++++++++++---
>>  qemu-timer.c                | 20 +++++++---
>>  target-arm/machine.c        |  4 +-
>>  30 files changed, 149 insertions(+), 45 deletions(-)
>>
>> -- 
>> 2.1.0
>>
>>
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 2/4] qemu-timer: add timer_init and timer_init_ns/us/ms
  2015-01-09  2:19   ` Fam Zheng
@ 2015-01-09 10:39     ` Paolo Bonzini
  0 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2015-01-09 10:39 UTC (permalink / raw)
  To: Fam Zheng; +Cc: qemu-devel



On 09/01/2015 03:19, Fam Zheng wrote:
>> >  /**
>> > + * timer_init:
>> > + * @clock: the clock to associate with the timer
> s/@clock/@type/
> 
> And similarly below.

Doh, cut-and-paste error (it's also in timer_new and friends).

>> > + * @scale: the scale value for the timer
>> > + * @cb: the callback to call when the timer expires
>> > + * @opaque: the opaque pointer to pass to the callback
>> > + *
>> > + * Create a new timer with the given scale on the default timer list
> s/Create a new/Initialize a/ ?
> 
> And similarly below.

Good idea, thanks!

Paolo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 0/4] qemu-timer: introduce usable pointer-free API
  2015-01-09  9:07   ` Paolo Bonzini
@ 2015-01-10  1:35     ` Fam Zheng
  0 siblings, 0 replies; 12+ messages in thread
From: Fam Zheng @ 2015-01-10  1:35 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

On Fri, 01/09 10:07, Paolo Bonzini wrote:
> 
> 
> On 09/01/2015 03:10, Fam Zheng wrote:
> > On Thu, 01/08 11:03, Paolo Bonzini wrote:
> >> The current pointer free API for timers is very low level.  Introduce
> >> a new API that matches timer_new_ns/us/ms and also a new API timer_deinit
> >> that can be used instead of timer_free.
> >>
> >> Finally, mechanically change timer macro names in vmstate, to make it
> >> clear that they accept a pointer.
> >>
> >> This prepares for conversion of timers from QEMUTimer * to QEMUTimer.
> > 
> > Novice question: what's the advantage, please?
> 
> No need to free them, so no memory leaks.  From a quick look I found two
> (hw/net/pcnet.c and hw/char/serial.c).
> 
> Also it matches more recent device model APIs like memory regions or
> throttling.

Get it! Thanks.

Fam

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 4/4] vmstate: accept QEMUTimer in VMSTATE_TIMER*, add VMSTATE_TIMER_PTR*
  2015-01-08 10:03 ` [Qemu-devel] [PATCH 4/4] vmstate: accept QEMUTimer in VMSTATE_TIMER*, add VMSTATE_TIMER_PTR* Paolo Bonzini
@ 2015-01-15  9:04   ` Amit Shah
  2015-01-15  9:14     ` Paolo Bonzini
  0 siblings, 1 reply; 12+ messages in thread
From: Amit Shah @ 2015-01-15  9:04 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

On (Thu) 08 Jan 2015 [11:03:28], Paolo Bonzini wrote:
> Old users of VMSTATE_TIMER* are mechanically changed to VMSTATE_TIMER_PTR
> variants.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>


> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index e45fc49..55ba584 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -642,17 +642,29 @@ extern const VMStateInfo vmstate_info_bitmap;
>  #define VMSTATE_FLOAT64(_f, _s)                                       \
>      VMSTATE_FLOAT64_V(_f, _s, 0)
>  
> -#define VMSTATE_TIMER_TEST(_f, _s, _test)                             \
> +#define VMSTATE_TIMER_PTR_TEST(_f, _s, _test)                             \
>      VMSTATE_POINTER_TEST(_f, _s, _test, vmstate_info_timer, QEMUTimer *)
>  
> -#define VMSTATE_TIMER_V(_f, _s, _v)                                   \
> +#define VMSTATE_TIMER_PTR_V(_f, _s, _v)                                   \
>      VMSTATE_POINTER(_f, _s, _v, vmstate_info_timer, QEMUTimer *)
>  
> +#define VMSTATE_TIMER_PTR(_f, _s)                                         \
> +    VMSTATE_TIMER_PTR_V(_f, _s, 0)
> +
> +#define VMSTATE_TIMER_PTR_ARRAY(_f, _s, _n)                              \
> +    VMSTATE_ARRAY_OF_POINTER(_f, _s, _n, 0, vmstate_info_timer, QEMUTimer *)
> +
> +#define VMSTATE_TIMER_TEST(_f, _s, _test)                             \
> +    VMSTATE_SINGLE_TEST(_f, _s, _test, 0, vmstate_info_timer, QEMUTimer)
> +
> +#define VMSTATE_TIMER_V(_f, _s, _v)                                   \
> +    VMSTATE_SINGLE(_f, _s, _v, vmstate_info_timer, QEMUTimer)
> +
>  #define VMSTATE_TIMER(_f, _s)                                         \
>      VMSTATE_TIMER_V(_f, _s, 0)

Why leave this around?

		Amit

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 4/4] vmstate: accept QEMUTimer in VMSTATE_TIMER*, add VMSTATE_TIMER_PTR*
  2015-01-15  9:04   ` Amit Shah
@ 2015-01-15  9:14     ` Paolo Bonzini
  0 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2015-01-15  9:14 UTC (permalink / raw)
  To: Amit Shah; +Cc: qemu-devel



On 15/01/2015 10:04, Amit Shah wrote:
> 
>> > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
>> > index e45fc49..55ba584 100644
>> > --- a/include/migration/vmstate.h
>> > +++ b/include/migration/vmstate.h
>> > @@ -642,17 +642,29 @@ extern const VMStateInfo vmstate_info_bitmap;
>> >  #define VMSTATE_FLOAT64(_f, _s)                                       \
>> >      VMSTATE_FLOAT64_V(_f, _s, 0)
>> >  
>> > -#define VMSTATE_TIMER_TEST(_f, _s, _test)                             \
>> > +#define VMSTATE_TIMER_PTR_TEST(_f, _s, _test)                             \
>> >      VMSTATE_POINTER_TEST(_f, _s, _test, vmstate_info_timer, QEMUTimer *)
>> >  
>> > -#define VMSTATE_TIMER_V(_f, _s, _v)                                   \
>> > +#define VMSTATE_TIMER_PTR_V(_f, _s, _v)                                   \
>> >      VMSTATE_POINTER(_f, _s, _v, vmstate_info_timer, QEMUTimer *)
>> >  
>> > +#define VMSTATE_TIMER_PTR(_f, _s)                                         \
>> > +    VMSTATE_TIMER_PTR_V(_f, _s, 0)
>> > +
>> > +#define VMSTATE_TIMER_PTR_ARRAY(_f, _s, _n)                              \
>> > +    VMSTATE_ARRAY_OF_POINTER(_f, _s, _n, 0, vmstate_info_timer, QEMUTimer *)
>> > +
>> > +#define VMSTATE_TIMER_TEST(_f, _s, _test)                             \
>> > +    VMSTATE_SINGLE_TEST(_f, _s, _test, 0, vmstate_info_timer, QEMUTimer)
>> > +
>> > +#define VMSTATE_TIMER_V(_f, _s, _v)                                   \
>> > +    VMSTATE_SINGLE(_f, _s, _v, vmstate_info_timer, QEMUTimer)
>> > +
>> >  #define VMSTATE_TIMER(_f, _s)                                         \
>> >      VMSTATE_TIMER_V(_f, _s, 0)
> Why leave this around?

Because further changes down the line will convert QEMUTimer* to
QEMUTimer and thus go back to VMSTATE_TIMER.  Having both around avoids
having a single patch with -1000/+1000 diffstat.

Paolo

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-01-15  9:15 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-08 10:03 [Qemu-devel] [PATCH 0/4] qemu-timer: introduce usable pointer-free API Paolo Bonzini
2015-01-08 10:03 ` [Qemu-devel] [PATCH 1/4] qemu-timer: rename timer_init to timer_init_tl Paolo Bonzini
2015-01-08 10:03 ` [Qemu-devel] [PATCH 2/4] qemu-timer: add timer_init and timer_init_ns/us/ms Paolo Bonzini
2015-01-09  2:19   ` Fam Zheng
2015-01-09 10:39     ` Paolo Bonzini
2015-01-08 10:03 ` [Qemu-devel] [PATCH 3/4] qemu-timer: introduce timer_deinit Paolo Bonzini
2015-01-08 10:03 ` [Qemu-devel] [PATCH 4/4] vmstate: accept QEMUTimer in VMSTATE_TIMER*, add VMSTATE_TIMER_PTR* Paolo Bonzini
2015-01-15  9:04   ` Amit Shah
2015-01-15  9:14     ` Paolo Bonzini
2015-01-09  2:10 ` [Qemu-devel] [PATCH 0/4] qemu-timer: introduce usable pointer-free API Fam Zheng
2015-01-09  9:07   ` Paolo Bonzini
2015-01-10  1:35     ` Fam Zheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).