Daniel P Berrange writes: > This converts the HMP/QMP monitor API implementations > and some internal trace control methods to use the new > trace event iterator APIs.
> Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com> > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> > --- > monitor.c | 26 ++++++++++-------- > trace/control.c | 85 > +++++++++++++++++++++++++++++++-------------------------- > trace/qmp.c | 16 +++++++---- > 3 files changed, 70 insertions(+), 57 deletions(-) > diff --git a/monitor.c b/monitor.c > index 8bb8bbf..129d37e 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -3327,13 +3327,14 @@ void info_trace_events_completion(ReadLineState *rs, > int nb_args, const char *st > len = strlen(str); > readline_set_completion_index(rs, len); > if (nb_args == 2) { > - TraceEventID id; > - for (id = 0; id < trace_event_count(); id++) { > - const char *event_name = > trace_event_get_name(trace_event_id(id)); > - if (!strncmp(str, event_name, len)) { > - readline_add_completion(rs, event_name); > - } > + TraceEventIter iter; > + TraceEvent *ev; > + char *pattern = g_strdup_printf("%s*", str); > + trace_event_iter_init(&iter, pattern); > + while ((ev = trace_event_iter_next(&iter)) != NULL) { > + readline_add_completion(rs, trace_event_get_name(ev)); > } > + g_free(pattern); > } > } > @@ -3344,13 +3345,14 @@ void trace_event_completion(ReadLineState *rs, int > nb_args, const char *str) > len = strlen(str); > readline_set_completion_index(rs, len); > if (nb_args == 2) { > - TraceEventID id; > - for (id = 0; id < trace_event_count(); id++) { > - const char *event_name = > trace_event_get_name(trace_event_id(id)); > - if (!strncmp(str, event_name, len)) { > - readline_add_completion(rs, event_name); > - } > + TraceEventIter iter; > + TraceEvent *ev; > + char *pattern = g_strdup_printf("%s*", str); > + trace_event_iter_init(&iter, pattern); > + while ((ev = trace_event_iter_next(&iter)) != NULL) { > + readline_add_completion(rs, trace_event_get_name(ev)); > } > + g_free(pattern); > } else if (nb_args == 3) { > add_completion_option(rs, str, "on"); > add_completion_option(rs, str, "off"); > diff --git a/trace/control.c b/trace/control.c > index 1a96049..653b70c 100644 > --- a/trace/control.c > +++ b/trace/control.c > @@ -60,9 +60,10 @@ TraceEvent *trace_event_name(const char *name) > { > assert(name != NULL); > - TraceEventID i; > - for (i = 0; i < trace_event_count(); i++) { > - TraceEvent *ev = trace_event_id(i); > + TraceEventIter iter; > + TraceEvent *ev; > + trace_event_iter_init(&iter, NULL); > + while ((ev = trace_event_iter_next(&iter)) != NULL) { > if (strcmp(trace_event_get_name(ev), name) == 0) { > return ev; > } > @@ -105,21 +106,18 @@ TraceEvent *trace_event_pattern(const char *pat, > TraceEvent *ev) > { > assert(pat != NULL); > - TraceEventID i; > - > - if (ev == NULL) { > - i = -1; > - } else { > - i = trace_event_get_id(ev); > - } > - i++; > - > - while (i < trace_event_count()) { > - TraceEvent *res = trace_event_id(i); > - if (pattern_glob(pat, trace_event_get_name(res))) { > - return res; > + bool matched = ev ? false : true; > + TraceEventIter iter; > + TraceEvent *thisev; > + trace_event_iter_init(&iter, pat); > + while ((thisev = trace_event_iter_next(&iter)) != NULL) { > + if (matched) { > + return thisev; > + } else { > + if (ev == thisev) { > + matched = true; > + } > } > - i++; > } > return NULL; > @@ -148,10 +146,11 @@ TraceEvent *trace_event_iter_next(TraceEventIter *iter) > void trace_list_events(void) > { > - int i; > - for (i = 0; i < trace_event_count(); i++) { > - TraceEvent *res = trace_event_id(i); > - fprintf(stderr, "%s\n", trace_event_get_name(res)); > + TraceEventIter iter; > + TraceEvent *ev; > + trace_event_iter_init(&iter, NULL); > + while ((ev = trace_event_iter_next(&iter)) != NULL) { > + fprintf(stderr, "%s\n", trace_event_get_name(ev)); > } > } > @@ -159,26 +158,32 @@ static void do_trace_enable_events(const char *line_buf) > { > const bool enable = ('-' != line_buf[0]); > const char *line_ptr = enable ? line_buf : line_buf + 1; > - > - if (trace_event_is_pattern(line_ptr)) { > - TraceEvent *ev = NULL; > - while ((ev = trace_event_pattern(line_ptr, ev)) != NULL) { > - if (trace_event_get_state_static(ev)) { > - trace_event_set_state_dynamic_init(ev, enable); > + TraceEventIter iter; > + TraceEvent *ev; > + bool is_pattern = trace_event_is_pattern(line_ptr); > + > + trace_event_iter_init(&iter, line_ptr); > + while ((ev = trace_event_iter_next(&iter)) != NULL) { > + if (!trace_event_get_state_static(ev)) { > + if (!is_pattern) { > + error_report("WARNING: trace event '%s' is not traceable", > + line_ptr); > + return; > } > + continue; > } > - } else { > - TraceEvent *ev = trace_event_name(line_ptr); > - if (ev == NULL) { > - error_report("WARNING: trace event '%s' does not exist", > - line_ptr); > - } else if (!trace_event_get_state_static(ev)) { > - error_report("WARNING: trace event '%s' is not traceable", > - line_ptr); > - } else { > - trace_event_set_state_dynamic_init(ev, enable); > + > + /* start tracing */ > + trace_event_set_state_dynamic(ev, enable); > + if (!is_pattern) { > + break; > } > } > + > + if (!is_pattern) { > + error_report("WARNING: trace event '%s' does not exist", > + line_ptr); > + } > } > void trace_enable_events(const char *line_buf) > @@ -293,8 +298,10 @@ char *trace_opt_parse(const char *optarg) > void trace_init_vcpu_events(void) > { > - TraceEvent *ev = NULL; > - while ((ev = trace_event_pattern("*", ev)) != NULL) { > + TraceEventIter iter; > + TraceEvent *ev; > + trace_event_iter_init(&iter, NULL); > + while ((ev = trace_event_iter_next(&iter)) != NULL) { > if (trace_event_is_vcpu(ev) && > trace_event_get_state_static(ev) && > trace_event_get_state_dynamic(ev)) { > diff --git a/trace/qmp.c b/trace/qmp.c > index 11d2564..88a907b 100644 > --- a/trace/qmp.c > +++ b/trace/qmp.c > @@ -52,8 +52,10 @@ static bool check_events(bool has_vcpu, bool > ignore_unavailable, bool is_pattern > return true; > } else { > /* error for unavailable events */ > - TraceEvent *ev = NULL; > - while ((ev = trace_event_pattern(name, ev)) != NULL) { > + TraceEventIter iter; > + TraceEvent *ev; > + trace_event_iter_init(&iter, name); > + while ((ev = trace_event_iter_next(&iter)) != NULL) { > if (!ignore_unavailable && !trace_event_get_state_static(ev)) { > error_setg(errp, "event \"%s\" is disabled", > trace_event_get_name(ev)); > return false; > @@ -69,6 +71,7 @@ TraceEventInfoList *qmp_trace_event_get_state(const char > *name, > { > Error *err = NULL; > TraceEventInfoList *events = NULL; > + TraceEventIter iter; > TraceEvent *ev; > bool is_pattern = trace_event_is_pattern(name); > CPUState *cpu; > @@ -86,8 +89,8 @@ TraceEventInfoList *qmp_trace_event_get_state(const char > *name, > } > /* Get states (all errors checked above) */ > - ev = NULL; > - while ((ev = trace_event_pattern(name, ev)) != NULL) { > + trace_event_iter_init(&iter, is_pattern ? name : NULL); I think you should always pass name, otherwise a non-pattern name (when the user wants an exact name match) will return the state of all events. > + while ((ev = trace_event_iter_next(&iter)) != NULL) { > TraceEventInfoList *elem; > bool is_vcpu = trace_event_is_vcpu(ev); > if (has_vcpu && !is_vcpu) { > @@ -132,6 +135,7 @@ void qmp_trace_event_set_state(const char *name, bool > enable, > Error **errp) > { > Error *err = NULL; > + TraceEventIter iter; > TraceEvent *ev; > bool is_pattern = trace_event_is_pattern(name); > CPUState *cpu; > @@ -150,8 +154,8 @@ void qmp_trace_event_set_state(const char *name, bool > enable, > } > /* Apply changes (all errors checked above) */ > - ev = NULL; > - while ((ev = trace_event_pattern(name, ev)) != NULL) { > + trace_event_iter_init(&iter, name); > + while ((ev = trace_event_iter_next(&iter)) != NULL) { > if (!trace_event_get_state_static(ev) || > (has_vcpu && !trace_event_is_vcpu(ev))) { > continue; > -- > 2.7.4 Thanks, Lluis