[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/6] trace: add trace event iterator APIs
From: |
Lluís Vilanova |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/6] trace: add trace event iterator APIs |
Date: |
Wed, 14 Sep 2016 23:53:10 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Daniel P Berrange writes:
> Currently methods which want to iterate over trace events,
> do so using the trace_event_count() and trace_event_id()
> methods. This leaks the concept of a single ID enum to
> the callers. There is an alternative trace_event_pattern()
> method which can be used in an iteration context, but its
> design is stateless, so is not easy to expand it in the
> future.
> This defines a formal iterator API will provide an future
> proof way of iterating over events.
> The iterator is also able to apply a pattern match filter
> to events, further removing the need for the pattern
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
> trace/control.c | 20 ++++++++++++++++++++
> trace/control.h | 27 +++++++++++++++++++++++++++
> 2 files changed, 47 insertions(+)
> diff --git a/trace/control.c b/trace/control.c
> index 05d85ac..b871727 100644
> --- a/trace/control.c
> +++ b/trace/control.c
> @@ -125,6 +125,26 @@ TraceEvent *trace_event_pattern(const char *pat,
> TraceEvent *ev)
> return NULL;
> }
> +void trace_event_iter_init(TraceEventIter *iter, const char *pattern)
> +{
> + iter->event = 0;
> + iter->pattern = pattern;
> +}
> +
> +TraceEvent *trace_event_iter_next(TraceEventIter *iter)
> +{
> + while (iter->event < TRACE_EVENT_COUNT) {
> + if (!iter->pattern ||
> + pattern_glob(iter->pattern,
> +
> trace_event_get_name(&(trace_events[iter->event])))) {
> + return &(trace_events[iter->event]);
That's a picky one (feel free to ignore), but can you refactor
"&(trace_events[iter->event])" out into a variable? The long pattern_glob() call
is a bit hard to parse.
> + }
> + iter->event++;
> + }
> +
> + return NULL;
> +}
> +
> void trace_list_events(void)
> {
> int i;
> diff --git a/trace/control.h b/trace/control.h
> index 27a16fc..c71b405 100644
> --- a/trace/control.h
> +++ b/trace/control.h
> @@ -13,6 +13,10 @@
> #include "qemu-common.h"
> #include "trace/generated-events.h"
> +typedef struct TraceEventIter {
> + size_t event;
Shouldn't this be TraceEventID for consistence with "trace/control.h"? But if
you're going to drop TraceEventID in a later series feel free to ignore me.
Other than those two, the rest looks good to me.
Cheers,
Lluis
- [Qemu-devel] [PATCH v2 0/6] Prep changes for modular trace-events build, Daniel P. Berrange, 2016/09/14
- [Qemu-devel] [PATCH v2 1/6] trace: add trace event iterator APIs, Daniel P. Berrange, 2016/09/14
- Re: [Qemu-devel] [PATCH v2 1/6] trace: add trace event iterator APIs,
Lluís Vilanova <=
- [Qemu-devel] [PATCH v2 3/6] trace: remove some now unused functions, Daniel P. Berrange, 2016/09/14
- [Qemu-devel] [PATCH v2 2/6] trace: convert code to use event iterators, Daniel P. Berrange, 2016/09/14
- [Qemu-devel] [PATCH v2 4/6] trace: remove global 'uint16 dstate[]' array, Daniel P. Berrange, 2016/09/14
- [Qemu-devel] [PATCH v2 6/6] trace: use -1 instead of TRACE_VCPU_EVENT_COUNT as magic value, Daniel P. Berrange, 2016/09/14