diff mbox series

[v2,2/4] Introduce attributes to qemu timer subsystem

Message ID f47b81dbce734e9806f9516eba8ca588e6321c2f.1539764043.git.artem.k.pisarenko@gmail.com (mailing list archive)
State New, archived
Headers show
Series Introduce attributes for timers subsystem and remove QEMU_CLOCK_VIRTUAL_EXT clock type | expand

Commit Message

Artem Pisarenko Oct. 17, 2018, 8:24 a.m. UTC
Attributes are simple flags, associated with individual timers for their whole lifetime.
They intended to be used to mark individual timers for special handling by various qemu features operating at qemu core level.
New/init functions family in timer interface updated and refactored (new 'attribute' argument added, timer_list replaced with timer_list_group+type combinations, comments improved to avoid info duplication).
Also existing aio interface extended with attribute-enabled variants of functions, which create/initialize timers.

Signed-off-by: Artem Pisarenko <artem.k.pisarenko@gmail.com>
---

Notes:
    v2:
    - timer creation/initialize functions reworked and and their unnecessary variants removed (as Paolo Bonzini suggested)
    - also their comments improved to avoid info duplication

 include/block/aio.h       |  57 ++++++++++++++++++---
 include/qemu/timer.h      | 128 ++++++++++++++++++++++++++--------------------
 tests/ptimer-test-stubs.c |  13 +++--
 util/qemu-timer.c         |  18 +++++--
 4 files changed, 146 insertions(+), 70 deletions(-)

Comments

Stefan Hajnoczi Oct. 17, 2018, 9:12 a.m. UTC | #1
On Wed, Oct 17, 2018 at 02:24:19PM +0600, Artem Pisarenko wrote:
> Attributes are simple flags, associated with individual timers for their whole lifetime.
> They intended to be used to mark individual timers for special handling by various qemu features operating at qemu core level.

I'm worried that this sentence suggests various parts of QEMU will stash
state in ts->attributes.  That's messy and they shouldn't do this.  Make
the field private to qemu-timer.c.

Attributes should only affect qemu-timer.c behavior.  Other parts of
QEMU should not act differently based on timer attributes (i.e. checking
bits).  If they need to do that then it suggests something isn't
properly encapsulated in qemu-timer.c.

> diff --git a/include/block/aio.h b/include/block/aio.h
> index f08630c..fce9d48 100644
> --- a/include/block/aio.h
> +++ b/include/block/aio.h
> @@ -388,6 +388,31 @@ struct LinuxAioState *aio_setup_linux_aio(AioContext *ctx, Error **errp);
>  struct LinuxAioState *aio_get_linux_aio(AioContext *ctx);
>  
>  /**
> + * aio_timer_new_with_attrs:
> + * @ctx: the aio context
> + * @type: the clock type
> + * @scale: the scale
> + * @attributes: 0, or one to multiple OR'ed QEMU_TIMER_ATTR(id) values to assign

Further down in this patch the notation is QEMU_TIMER_ATTR_<id>, which I
think is clearer because QEMU_TIMER_ATTR(id) looks like a (non-existent)
macro.  Please use the QEMU_TIMER_ATTR_<id> notation consistently.

> +/**
> + * QEMU Timer attributes:
> + *
> + * An individual timer may be assigned with one or multiple attributes when
> + * initialized.
> + * Attribute is a static flag, meaning that timer has corresponding property.
> + * Attributes are defined in QEMUTimerAttrBit enum and encoded to bit set,
> + * which used to initialize timer, stored to 'attributes' member and can be
> + * retrieved externally with timer_get_attributes() call.
> + * Values of QEMUTimerAttrBit aren't used directly,
> + * instead each attribute in bit set accessed with QEMU_TIMER_ATTR_<id> macro,
> + * where <id> is a unique part of attribute identifier.
> + *
> + * No attributes defined currently.
> + */
> +
> +typedef enum {
> +    QEMU_TIMER_ATTRBIT__NONE
> +} QEMUTimerAttrBit;
> +
> +#define QEMU_TIMER_ATTR__NONE (1 << QEMU_TIMER_ATTRBIT__NONE)

What is the purpose of this bit?  I guess it's just here as a
placeholder because no real bits have been defined yet.  Hopefully the
next patch removes it (/* This placeholder is removed in the next patch
*/ would be a nice way to document this for reviewers).

The enum isn't needed and makes debugging harder since the bit number is
implicit in the enum ordering.  This alternative is clearer and more
concise:

  #define QEMU_TIMER_ATTR_foo BIT(n)
Paolo Bonzini Oct. 17, 2018, 9:15 a.m. UTC | #2
On 17/10/2018 11:12, Stefan Hajnoczi wrote:
>> Attributes are simple flags, associated with individual timers for their whole lifetime.
>> They intended to be used to mark individual timers for special handling by various qemu features operating at qemu core level.
> I'm worried that this sentence suggests various parts of QEMU will stash
> state in ts->attributes.  That's messy and they shouldn't do this.  Make
> the field private to qemu-timer.c.

Yes, the contents of the fields are private.  Are you suggesting a
different wording for the commit message or the "QEMU Timer attributes"
doc comment, or something more than that?  Possibly removing
timer_get_attributes altogether?

Paolo
Artem Pisarenko Oct. 17, 2018, 10:57 a.m. UTC | #3
> Further down in this patch the notation is QEMU_TIMER_ATTR_<id>, which I
> think is clearer because QEMU_TIMER_ATTR(id) looks like a (non-existent)
> macro.  Please use the QEMU_TIMER_ATTR_<id> notation consistently.

Yes, I've just forgot to update comments after previous patch version,
where it actually was macro.

> What is the purpose of this bit?  I guess it's just here as a
> placeholder because no real bits have been defined yet.  Hopefully the
> next patch removes it (/* This placeholder is removed in the next patch
> */ would be a nice way to document this for reviewers).

It's just to prevent compilation errors, as required by
https://wiki.qemu.org/Contribute/SubmitAPatch#Split_up_long_patches

> The enum isn't needed and makes debugging harder since the bit number is
> implicit in the enum ordering.  This alternative is clearer and more
> concise:
>
>   #define QEMU_TIMER_ATTR_foo BIT(n)

Agree.

>ср, 17 окт. 2018 г. в 15:15, Paolo Bonzini <pbonzini@redhat.com>:
>On 17/10/2018 11:12, Stefan Hajnoczi wrote:
>>> Attributes are simple flags, associated with individual timers for
their whole lifetime.
>>> They intended to be used to mark individual timers for special handling
by various qemu features operating at qemu core level.
>> I'm worried that this sentence suggests various parts of QEMU will stash
>> state in ts->attributes.  That's messy and they shouldn't do this.  Make
>> the field private to qemu-timer.c.
>
> Yes, the contents of the fields are private.  Are you suggesting a
> different wording for the commit message or the "QEMU Timer attributes"
> doc comment, or something more than that?  Possibly removing
> timer_get_attributes altogether?

Actually, attributes aren't intended to be changed externally of timers
code.
The purpose of timer_get_attributes() is just informational, same like if
timer_get_clock(), timer_get_scale(), etc. would exist. But since none of
such accessors exists, adding just one for 'attributes' looks exceptional.
And it isn't being used even after whole patch series applied. As such, it
could be just removed.
Any suggestons to improve commit message and/or comments, if any ?
Paolo Bonzini Oct. 17, 2018, 11:24 a.m. UTC | #4
On 17/10/2018 12:57, Artem Pisarenko wrote:
>> Further down in this patch the notation is QEMU_TIMER_ATTR_<id>, which I
>> think is clearer because QEMU_TIMER_ATTR(id) looks like a (non-existent)
>> macro.  Please use the QEMU_TIMER_ATTR_<id> notation consistently.
> 
> Yes, I've just forgot to update comments after previous patch version,
> where it actually was macro.
> 
>> What is the purpose of this bit?  I guess it's just here as a
>> placeholder because no real bits have been defined yet.  Hopefully the
>> next patch removes it (/* This placeholder is removed in the next patch
>> */ would be a nice way to document this for reviewers).
> 
> It's just to prevent compilation errors, as required by
> https://wiki.qemu.org/Contribute/SubmitAPatch#Split_up_long_patches
> 
>> The enum isn't needed and makes debugging harder since the bit number is
>> implicit in the enum ordering.  This alternative is clearer and more
>> concise:
>> 
>>   #define QEMU_TIMER_ATTR_foo BIT(n)
> 
> Agree.

Like this?

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 86ce70f20e..ef7526e389 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -55,25 +55,13 @@ typedef enum {
 /**
  * QEMU Timer attributes:
  *
- * An individual timer may be assigned with one or multiple attributes when
- * initialized.
- * Attribute is a static flag, meaning that timer has corresponding
property.
- * Attributes are defined in QEMUTimerAttrBit enum and encoded to bit set,
- * which used to initialize timer, stored to 'attributes' member and can be
- * retrieved externally with timer_get_attributes() call.
- * Values of QEMUTimerAttrBit aren't used directly,
- * instead each attribute in bit set accessed with QEMU_TIMER_ATTR_<id>
macro,
- * where <id> is a unique part of attribute identifier.
+ * An individual timer may be given one or multiple attributes when
initialized.
+ * Each attribute corresponds to one bit.  Attributes modify the processing
+ * of timers when they fire.
  *
  * No attributes defined currently.
  */

-typedef enum {
-    QEMU_TIMER_ATTRBIT__NONE
-} QEMUTimerAttrBit;
-
-#define QEMU_TIMER_ATTR__NONE (1 << QEMU_TIMER_ATTRBIT__NONE)
-
 typedef struct QEMUTimerList QEMUTimerList;

 struct QEMUTimerListGroup {
@@ -640,14 +628,6 @@ static inline QEMUTimer *timer_new_ms(QEMUClockType
type, QEMUTimerCB *cb,
     return timer_new(type, SCALE_MS, cb, opaque);
 }

-/**
- * timer_get_attributes:
- * @ts: the timer
- *
- * Return 0, or one to multiple OR'ed QEMU_TIMER_ATTR(id) values
- */
-int timer_get_attributes(QEMUTimer *ts);
-
 /**
  * timer_deinit:
  * @ts: the timer to be de-initialised
diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index 2046b68c15..04527a343f 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -355,11 +355,6 @@ void timer_init_full(QEMUTimer *ts,
     ts->expire_time = -1;
 }

-int timer_get_attributes(QEMUTimer *ts)
-{
-    return ts->attributes;
-}
-
 void timer_deinit(QEMUTimer *ts)
 {
     assert(ts->expire_time == -1);
Artem Pisarenko Oct. 17, 2018, 1:07 p.m. UTC | #5
Yes, but without words ..."when they fire" in attributes comments.

ср, 17 окт. 2018 г. в 17:24, Paolo Bonzini <pbonzini@redhat.com>:

> On 17/10/2018 12:57, Artem Pisarenko wrote:
> >> Further down in this patch the notation is QEMU_TIMER_ATTR_<id>, which I
> >> think is clearer because QEMU_TIMER_ATTR(id) looks like a (non-existent)
> >> macro.  Please use the QEMU_TIMER_ATTR_<id> notation consistently.
> >
> > Yes, I've just forgot to update comments after previous patch version,
> > where it actually was macro.
> >
> >> What is the purpose of this bit?  I guess it's just here as a
> >> placeholder because no real bits have been defined yet.  Hopefully the
> >> next patch removes it (/* This placeholder is removed in the next patch
> >> */ would be a nice way to document this for reviewers).
> >
> > It's just to prevent compilation errors, as required by
> > https://wiki.qemu.org/Contribute/SubmitAPatch#Split_up_long_patches
> >
> >> The enum isn't needed and makes debugging harder since the bit number is
> >> implicit in the enum ordering.  This alternative is clearer and more
> >> concise:
> >>
> >>   #define QEMU_TIMER_ATTR_foo BIT(n)
> >
> > Agree.
>
> Like this?
>
> ...
>
> --

С уважением,
  Артем Писаренко
Paolo Bonzini Oct. 17, 2018, 2:43 p.m. UTC | #6
On 17/10/2018 15:07, Artem Pisarenko wrote:
> Yes, but without words ..."when they fire" in attributes comments.

For now the only attribute applies when timers fire; that sentence was a
way to clarify the intended scope and address Stefan's comment.  I can
remove it too, but I'd like to understand if you have other ideas for
attributes.

Paolo
Artem Pisarenko Oct. 17, 2018, 2:57 p.m. UTC | #7
>ср, 17 окт. 2018 г. в 20:43, Paolo Bonzini:
>On 17/10/2018 15:07, Artem Pisarenko wrote:
>> Yes, but without words ..."when they fire" in attributes comments.
>
> For now the only attribute applies when timers fire; that sentence was a
> way to clarify the intended scope and address Stefan's comment.  I can
> remove it too, but I'd like to understand if you have other ideas for
> attributes.

No, I haven't. I don't think that this partcular narrowing has any
relevance to Stefan's comment, so I just considered it to be unnecessary. I
don't insist.
Stefan Hajnoczi Oct. 18, 2018, 9:26 a.m. UTC | #8
On Wed, Oct 17, 2018 at 01:24:28PM +0200, Paolo Bonzini wrote:
> On 17/10/2018 12:57, Artem Pisarenko wrote:
> >> Further down in this patch the notation is QEMU_TIMER_ATTR_<id>, which I
> >> think is clearer because QEMU_TIMER_ATTR(id) looks like a (non-existent)
> >> macro.  Please use the QEMU_TIMER_ATTR_<id> notation consistently.
> > 
> > Yes, I've just forgot to update comments after previous patch version,
> > where it actually was macro.
> > 
> >> What is the purpose of this bit?  I guess it's just here as a
> >> placeholder because no real bits have been defined yet.  Hopefully the
> >> next patch removes it (/* This placeholder is removed in the next patch
> >> */ would be a nice way to document this for reviewers).
> > 
> > It's just to prevent compilation errors, as required by
> > https://wiki.qemu.org/Contribute/SubmitAPatch#Split_up_long_patches
> > 
> >> The enum isn't needed and makes debugging harder since the bit number is
> >> implicit in the enum ordering.  This alternative is clearer and more
> >> concise:
> >> 
> >>   #define QEMU_TIMER_ATTR_foo BIT(n)
> > 
> > Agree.
> 
> Like this?

Yes, something like that is good.
diff mbox series

Patch

diff --git a/include/block/aio.h b/include/block/aio.h
index f08630c..fce9d48 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -388,6 +388,31 @@  struct LinuxAioState *aio_setup_linux_aio(AioContext *ctx, Error **errp);
 struct LinuxAioState *aio_get_linux_aio(AioContext *ctx);
 
 /**
+ * aio_timer_new_with_attrs:
+ * @ctx: the aio context
+ * @type: the clock type
+ * @scale: the scale
+ * @attributes: 0, or one to multiple OR'ed QEMU_TIMER_ATTR(id) values to assign
+ * @cb: the callback to call on timer expiry
+ * @opaque: the opaque pointer to pass to the callback
+ *
+ * Allocate a new timer (with attributes) attached to the context @ctx.
+ * The function is responsible for memory allocation.
+ *
+ * The preferred interface is aio_timer_init or aio_timer_init_with_attrs.
+ * Use that unless you really need dynamic memory allocation.
+ *
+ * Returns: a pointer to the new timer
+ */
+static inline QEMUTimer *aio_timer_new_with_attrs(AioContext *ctx,
+                                                  QEMUClockType type,
+                                                  int scale, int attributes,
+                                                  QEMUTimerCB *cb, void *opaque)
+{
+    return timer_new_full(&ctx->tlg, type, scale, attributes, cb, opaque);
+}
+
+/**
  * aio_timer_new:
  * @ctx: the aio context
  * @type: the clock type
@@ -396,10 +421,7 @@  struct LinuxAioState *aio_get_linux_aio(AioContext *ctx);
  * @opaque: the opaque pointer to pass to the callback
  *
  * Allocate a new timer attached to the context @ctx.
- * The function is responsible for memory allocation.
- *
- * The preferred interface is aio_timer_init. Use that
- * unless you really need dynamic memory allocation.
+ * See aio_timer_new_with_attrs for details.
  *
  * Returns: a pointer to the new timer
  */
@@ -407,7 +429,28 @@  static inline QEMUTimer *aio_timer_new(AioContext *ctx, QEMUClockType type,
                                        int scale,
                                        QEMUTimerCB *cb, void *opaque)
 {
-    return timer_new_tl(ctx->tlg.tl[type], scale, cb, opaque);
+    return timer_new_full(&ctx->tlg, type, scale, 0, cb, opaque);
+}
+
+/**
+ * aio_timer_init_with_attrs:
+ * @ctx: the aio context
+ * @ts: the timer
+ * @type: the clock type
+ * @scale: the scale
+ * @attributes: 0, or one to multiple OR'ed QEMU_TIMER_ATTR(id) values to assign
+ * @cb: the callback to call on timer expiry
+ * @opaque: the opaque pointer to pass to the callback
+ *
+ * Initialise a new timer (with attributes) attached to the context @ctx.
+ * The caller is responsible for memory allocation.
+ */
+static inline void aio_timer_init_with_attrs(AioContext *ctx,
+                                             QEMUTimer *ts, QEMUClockType type,
+                                             int scale, int attributes,
+                                             QEMUTimerCB *cb, void *opaque)
+{
+    timer_init_full(ts, &ctx->tlg, type, scale, attributes, cb, opaque);
 }
 
 /**
@@ -420,14 +463,14 @@  static inline QEMUTimer *aio_timer_new(AioContext *ctx, QEMUClockType type,
  * @opaque: the opaque pointer to pass to the callback
  *
  * Initialise a new timer attached to the context @ctx.
- * The caller is responsible for memory allocation.
+ * See aio_timer_init_with_attrs for details.
  */
 static inline void aio_timer_init(AioContext *ctx,
                                   QEMUTimer *ts, QEMUClockType type,
                                   int scale,
                                   QEMUTimerCB *cb, void *opaque)
 {
-    timer_init_tl(ts, ctx->tlg.tl[type], scale, cb, opaque);
+    timer_init_full(ts, &ctx->tlg, type, scale, 0, cb, opaque);
 }
 
 /**
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 39ea907..e225ad4 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -52,6 +52,28 @@  typedef enum {
     QEMU_CLOCK_MAX
 } QEMUClockType;
 
+/**
+ * QEMU Timer attributes:
+ *
+ * An individual timer may be assigned with one or multiple attributes when
+ * initialized.
+ * Attribute is a static flag, meaning that timer has corresponding property.
+ * Attributes are defined in QEMUTimerAttrBit enum and encoded to bit set,
+ * which used to initialize timer, stored to 'attributes' member and can be
+ * retrieved externally with timer_get_attributes() call.
+ * Values of QEMUTimerAttrBit aren't used directly,
+ * instead each attribute in bit set accessed with QEMU_TIMER_ATTR_<id> macro,
+ * where <id> is a unique part of attribute identifier.
+ *
+ * No attributes defined currently.
+ */
+
+typedef enum {
+    QEMU_TIMER_ATTRBIT__NONE
+} QEMUTimerAttrBit;
+
+#define QEMU_TIMER_ATTR__NONE (1 << QEMU_TIMER_ATTRBIT__NONE)
+
 typedef struct QEMUTimerList QEMUTimerList;
 
 struct QEMUTimerListGroup {
@@ -67,6 +89,7 @@  struct QEMUTimer {
     QEMUTimerCB *cb;
     void *opaque;
     QEMUTimer *next;
+    int attributes;
     int scale;
 };
 
@@ -418,22 +441,27 @@  int64_t timerlistgroup_deadline_ns(QEMUTimerListGroup *tlg);
  */
 
 /**
- * timer_init_tl:
+ * timer_init_full:
  * @ts: the timer to be initialised
- * @timer_list: the timer list to attach the timer to
+ * @timer_list_group: (optional) the timer list group to attach the timer to
+ * @type: the clock type to use
  * @scale: the scale value for the timer
+ * @attributes: 0, or one to multiple OR'ed QEMU_TIMER_ATTR(id) values to assign
  * @cb: the callback to be called when the timer expires
  * @opaque: the opaque pointer to be passed to the callback
  *
- * Initialise a new timer and associate it with @timer_list.
+ * Initialise a timer with the given scale and attributes,
+ * and associate it with timer list for given clock @type in @timer_list_group
+ * (or default timer list group, if NULL).
  * The caller is responsible for allocating the memory.
  *
  * You need not call an explicit deinit call. Simply make
  * sure it is not on a list with timer_del.
  */
-void timer_init_tl(QEMUTimer *ts,
-                   QEMUTimerList *timer_list, int scale,
-                   QEMUTimerCB *cb, void *opaque);
+void timer_init_full(QEMUTimer *ts,
+                     QEMUTimerListGroup *timer_list_group, QEMUClockType type,
+                     int scale, int attributes,
+                     QEMUTimerCB *cb, void *opaque);
 
 /**
  * timer_init:
@@ -445,14 +473,12 @@  void timer_init_tl(QEMUTimer *ts,
  *
  * Initialize a 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.
+ * See timer_init_full for details.
  */
 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_full(ts, &main_loop_tlg, type, scale, 0, cb, opaque);
 }
 
 /**
@@ -464,9 +490,7 @@  static inline void timer_init(QEMUTimer *ts, QEMUClockType type, int scale,
  *
  * Initialize a 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.
+ * See timer_init_full for details.
  */
 static inline void timer_init_ns(QEMUTimer *ts, QEMUClockType type,
                                  QEMUTimerCB *cb, void *opaque)
@@ -483,9 +507,7 @@  static inline void timer_init_ns(QEMUTimer *ts, QEMUClockType type,
  *
  * Initialize a 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.
+ * See timer_init_full for details.
  */
 static inline void timer_init_us(QEMUTimer *ts, QEMUClockType type,
                                  QEMUTimerCB *cb, void *opaque)
@@ -502,9 +524,7 @@  static inline void timer_init_us(QEMUTimer *ts, QEMUClockType type,
  *
  * Initialize a 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.
+ * See timer_init_full for details.
  */
 static inline void timer_init_ms(QEMUTimer *ts, QEMUClockType type,
                                  QEMUTimerCB *cb, void *opaque)
@@ -513,27 +533,37 @@  static inline void timer_init_ms(QEMUTimer *ts, QEMUClockType type,
 }
 
 /**
- * timer_new_tl:
- * @timer_list: the timer list to attach the timer to
+ * timer_new_full:
+ * @timer_list_group: (optional) the timer list group to attach the timer to
+ * @type: the clock type to use
  * @scale: the scale value for the timer
+ * @attributes: 0, or one to multiple OR'ed QEMU_TIMER_ATTR(id) values to assign
  * @cb: the callback to be called when the timer expires
  * @opaque: the opaque pointer to be passed to the callback
  *
- * Create a new timer and associate it with @timer_list.
+ * Create a new timer with the given scale and attributes,
+ * and associate it with timer list for given clock @type in @timer_list_group
+ * (or default timer list group, if NULL).
  * The memory is allocated by the function.
  *
  * This is not the preferred interface unless you know you
- * are going to call timer_free. Use timer_init instead.
+ * are going to call timer_free. Use timer_init or timer_init_full instead.
+ *
+ * The default timer list has one special feature: in icount mode,
+ * %QEMU_CLOCK_VIRTUAL timers are run in the vCPU thread.  This is
+ * not true of other timer lists, which are typically associated
+ * with an AioContext---each of them runs its timer callbacks in its own
+ * AioContext thread.
  *
  * Returns: a pointer to the timer
  */
-static inline QEMUTimer *timer_new_tl(QEMUTimerList *timer_list,
-                                      int scale,
-                                      QEMUTimerCB *cb,
-                                      void *opaque)
+static inline QEMUTimer *timer_new_full(QEMUTimerListGroup *timer_list_group,
+                                        QEMUClockType type,
+                                        int scale, int attributes,
+                                        QEMUTimerCB *cb, void *opaque)
 {
     QEMUTimer *ts = g_malloc0(sizeof(QEMUTimer));
-    timer_init_tl(ts, timer_list, scale, cb, opaque);
+    timer_init_full(ts, timer_list_group, type, scale, attributes, cb, opaque);
     return ts;
 }
 
@@ -544,21 +574,16 @@  static inline QEMUTimer *timer_new_tl(QEMUTimerList *timer_list,
  * @cb: the callback to be called when the timer expires
  * @opaque: the opaque pointer to be passed to the callback
  *
- * Create a new timer and associate it with the default
- * timer list for the clock type @type.
- *
- * The default timer list has one special feature: in icount mode,
- * %QEMU_CLOCK_VIRTUAL timers are run in the vCPU thread.  This is
- * not true of other timer lists, which are typically associated
- * with an AioContext---each of them runs its timer callbacks in its own
- * AioContext thread.
+ * Create a new timer with the given scale,
+ * and associate it with the default timer list for the clock type @type.
+ * See timer_new_full for details.
  *
  * Returns: a pointer to the timer
  */
 static inline QEMUTimer *timer_new(QEMUClockType type, int scale,
                                    QEMUTimerCB *cb, void *opaque)
 {
-    return timer_new_tl(main_loop_tlg.tl[type], scale, cb, opaque);
+    return timer_new_full(&main_loop_tlg, type, scale, 0, cb, opaque);
 }
 
 /**
@@ -569,12 +594,7 @@  static inline QEMUTimer *timer_new(QEMUClockType type, int scale,
  *
  * Create a new timer with nanosecond scale on the default timer list
  * associated with the clock.
- *
- * The default timer list has one special feature: in icount mode,
- * %QEMU_CLOCK_VIRTUAL timers are run in the vCPU thread.  This is
- * not true of other timer lists, which are typically associated
- * with an AioContext---each of them runs its timer callbacks in its own
- * AioContext thread.
+ * See timer_new_full for details.
  *
  * Returns: a pointer to the newly created timer
  */
@@ -590,14 +610,9 @@  static inline QEMUTimer *timer_new_ns(QEMUClockType type, QEMUTimerCB *cb,
  * @cb: the callback to call when the timer expires
  * @opaque: the opaque pointer to pass to the callback
  *
- * The default timer list has one special feature: in icount mode,
- * %QEMU_CLOCK_VIRTUAL timers are run in the vCPU thread.  This is
- * not true of other timer lists, which are typically associated
- * with an AioContext---each of them runs its timer callbacks in its own
- * AioContext thread.
- *
  * Create a new timer with microsecond scale on the default timer list
  * associated with the clock.
+ * See timer_new_full for details.
  *
  * Returns: a pointer to the newly created timer
  */
@@ -613,14 +628,9 @@  static inline QEMUTimer *timer_new_us(QEMUClockType type, QEMUTimerCB *cb,
  * @cb: the callback to call when the timer expires
  * @opaque: the opaque pointer to pass to the callback
  *
- * The default timer list has one special feature: in icount mode,
- * %QEMU_CLOCK_VIRTUAL timers are run in the vCPU thread.  This is
- * not true of other timer lists, which are typically associated
- * with an AioContext---each of them runs its timer callbacks in its own
- * AioContext thread.
- *
  * Create a new timer with millisecond scale on the default timer list
  * associated with the clock.
+ * See timer_new_full for details.
  *
  * Returns: a pointer to the newly created timer
  */
@@ -631,6 +641,14 @@  static inline QEMUTimer *timer_new_ms(QEMUClockType type, QEMUTimerCB *cb,
 }
 
 /**
+ * timer_get_attributes:
+ * @ts: the timer
+ *
+ * Return 0, or one to multiple OR'ed QEMU_TIMER_ATTR(id) values
+ */
+int timer_get_attributes(QEMUTimer *ts);
+
+/**
  * timer_deinit:
  * @ts: the timer to be de-initialised
  *
diff --git a/tests/ptimer-test-stubs.c b/tests/ptimer-test-stubs.c
index ca5cc3b..54b3fd2 100644
--- a/tests/ptimer-test-stubs.c
+++ b/tests/ptimer-test-stubs.c
@@ -34,14 +34,19 @@  int64_t ptimer_test_time_ns;
 int use_icount = 1;
 bool qtest_allowed;
 
-void timer_init_tl(QEMUTimer *ts,
-                   QEMUTimerList *timer_list, int scale,
-                   QEMUTimerCB *cb, void *opaque)
+void timer_init_full(QEMUTimer *ts,
+                     QEMUTimerListGroup *timer_list_group, QEMUClockType type,
+                     int scale, int attributes,
+                     QEMUTimerCB *cb, void *opaque)
 {
-    ts->timer_list = timer_list;
+    if (!timer_list_group) {
+        timer_list_group = &main_loop_tlg;
+    }
+    ts->timer_list = timer_list_group->tl[type];
     ts->cb = cb;
     ts->opaque = opaque;
     ts->scale = scale;
+    ts->attributes = attributes;
     ts->expire_time = -1;
 }
 
diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index 86bfe84..2046b68 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -339,17 +339,27 @@  int qemu_poll_ns(GPollFD *fds, guint nfds, int64_t timeout)
 }
 
 
-void timer_init_tl(QEMUTimer *ts,
-                   QEMUTimerList *timer_list, int scale,
-                   QEMUTimerCB *cb, void *opaque)
+void timer_init_full(QEMUTimer *ts,
+                     QEMUTimerListGroup *timer_list_group, QEMUClockType type,
+                     int scale, int attributes,
+                     QEMUTimerCB *cb, void *opaque)
 {
-    ts->timer_list = timer_list;
+    if (!timer_list_group) {
+        timer_list_group = &main_loop_tlg;
+    }
+    ts->timer_list = timer_list_group->tl[type];
     ts->cb = cb;
     ts->opaque = opaque;
     ts->scale = scale;
+    ts->attributes = attributes;
     ts->expire_time = -1;
 }
 
+int timer_get_attributes(QEMUTimer *ts)
+{
+    return ts->attributes;
+}
+
 void timer_deinit(QEMUTimer *ts)
 {
     assert(ts->expire_time == -1);