On 14/10/2018 16:55, 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. > Existing timer, aio and coroutine interface extended with attribute-enabled > variants of functions, which create/initialize timers. > > Signed-off-by: Artem Pisarenko <artem.k.pisare...@gmail.com> > --- > > Notes: > Conversion and association between QEMUTimerAttrBit and accessor macro > are dumb. > Maybe better alternatives exist (like QFlags in Qt framework) or existing > qemu code may be reused, if any. > Attributes also may be better named as flags, but they looks like > something volatile, whereas 'attribute' expresses constant nature better. > > include/block/aio.h | 50 +++++++++++++++++++- > include/qemu/coroutine.h | 5 +- > include/qemu/timer.h | 110 > ++++++++++++++++++++++++++++++++++++++------ > tests/ptimer-test-stubs.c | 7 +-- > util/qemu-coroutine-sleep.c | 6 ++- > util/qemu-timer.c | 12 +++-- > 6 files changed, 165 insertions(+), 25 deletions(-) > > diff --git a/include/block/aio.h b/include/block/aio.h > index f08630c..a6be3fb 100644 > --- a/include/block/aio.h > +++ b/include/block/aio.h > @@ -407,10 +407,35 @@ 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_a_tl(ctx->tlg.tl[type], scale, 0, cb, opaque); > }
The new function _a_tl is a bit ugly. I would prefer to: - only have a new timer_new_full that takes all of scale, attribute and timerlist - accept a NULL timerlist and default to main_loop_tlg.tl[type] - add aio_timer_{new,init}_with_attrs > diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h > index 9801e7f..cffc2b2 100644 > --- a/include/qemu/coroutine.h > +++ b/include/qemu/coroutine.h > @@ -276,7 +276,10 @@ void qemu_co_rwlock_unlock(CoRwlock *lock); > /** > * Yield the coroutine for a given duration > */ > -void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns); > +#define qemu_co_sleep_ns(type, ns) \ > + qemu_co_sleep_a_ns(type, 0, ns) > +void coroutine_fn qemu_co_sleep_a_ns(QEMUClockType type, int attributes, > + int64_t ns); I wouldn't bother adding co_sleep_a_ns unless it's needed. > /** > * Yield until a file descriptor becomes readable > diff --git a/include/qemu/timer.h b/include/qemu/timer.h > index 39ea907..031e3a1 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. I think QEMU_TIMER_ATTR is an unnecessary complication. I would just add a TIMER_ATTR_EXTERNAL constant. Paolo