Re: [Qemu-devel] [PATCH v2] Introduce attributes to qemu timer subsystem

2018-10-16 Thread Paolo Bonzini
On 16/10/2018 15:03, Artem Pisarenko wrote:
> +if (!timer_list) {
> +timer_list = main_loop_tlg.tl[type];
> +}
> +timer_init_full(ts, timer_list, scale, attributes, cb, opaque);

Please move this "if" to timer_init_full, so that here you can just pass
timer_list.  timer_init_full will then take a QEMUTimerListGroup* (NULL
defaults to _loop_tlg, aio_timer_new passes >tlg) and a
QEMUClockType instead of a QEMUTimerList*.

Paolo



[Qemu-devel] [PATCH v2] Introduce attributes to qemu timer subsystem

2018-10-16 Thread Artem Pisarenko
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 their 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 
---

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   |  53 ++-
 include/qemu/timer.h  | 126 ++
 tests/ptimer-test-stubs.c |   7 +--
 util/qemu-timer.c |  12 +++--
 4 files changed, 136 insertions(+), 62 deletions(-)

diff --git a/include/block/aio.h b/include/block/aio.h
index f08630c..07c291a 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -407,10 +407,38 @@ 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(type, ctx->tlg.tl[type],
+  scale, 0, cb, opaque);
 }
 
 /**
+ * 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. 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(type, ctx->tlg.tl[type],
+  scale, attributes, cb, opaque);
+}
+
+
+/**
  * aio_timer_init:
  * @ctx: the aio context
  * @ts: the timer
@@ -427,7 +455,28 @@ static inline void aio_timer_init(AioContext *ctx,
   int scale,
   QEMUTimerCB *cb, void *opaque)
 {
-timer_init_tl(ts, ctx->tlg.tl[type], scale, cb, opaque);
+timer_init_full(ts, ctx->tlg.tl[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.tl[type], scale, attributes, cb, opaque);
 }
 
 /**
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 39ea907..64a84ea 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_ macro,
+ * where  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,24 @@ int64_t