diff mbox

[v9,4/7] trace: [monitor] Use new event control interface

Message ID 20130110192318.29862.42.stgit@fimbulvetr.bsc.es
State New
Headers show

Commit Message

Lluís Vilanova Jan. 10, 2013, 7:23 p.m. UTC 
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 monitor.c |   20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

Comments

Stefan Hajnoczi Jan. 23, 2013, 3:30 p.m. UTC | #1 
On Thu, Jan 10, 2013 at 08:23:19PM +0100, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
>  monitor.c |   20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index 9cf419b..4c40541 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -735,10 +735,24 @@ static void do_trace_event_set_state(Monitor *mon, const QDict *qdict)
>  {
>      const char *tp_name = qdict_get_str(qdict, "name");
>      bool new_state = qdict_get_bool(qdict, "option");
> -    int ret = trace_event_set_state(tp_name, new_state);
>  
> -    if (!ret) {
> -        monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
> +    if (trace_event_is_pattern(tp_name)) {
> +        TraceEvent *ev = NULL;
> +        while ((ev = trace_event_pattern(tp_name, ev)) != NULL) {
> +            if (!trace_event_get_state_static(ev)) {
> +                monitor_printf(mon, "event \"%s\" is not traceable\n", tp_name);
> +            }
> +            trace_event_set_state_dynamic(ev, new_state);
> +        }
> +    } else {
> +        TraceEvent *ev = trace_event_name(tp_name);
> +        if (ev == NULL) {
> +            monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
> +        } else if (!trace_event_get_state_static(ev)) {
> +            monitor_printf(mon, "event \"%s\" is not traceable\n", tp_name);
> +        } else {
> +            trace_event_set_state_dynamic(ev, new_state);
> +        }

Do we need to duplicate the pattern vs not-a-pattern case?

We can loop with trace_event_pattern() and print the "unknown event
name" only if !trace_event_is_pattern().

Stefan
Lluís Vilanova Jan. 23, 2013, 5:01 p.m. UTC | #2 
Stefan Hajnoczi writes:

> On Thu, Jan 10, 2013 at 08:23:19PM +0100, Lluís Vilanova wrote:
>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>> ---
>> monitor.c |   20 +++++++++++++++++---
>> 1 file changed, 17 insertions(+), 3 deletions(-)
>> 
>> diff --git a/monitor.c b/monitor.c
>> index 9cf419b..4c40541 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -735,10 +735,24 @@ static void do_trace_event_set_state(Monitor *mon, const QDict *qdict)
>> {
>> const char *tp_name = qdict_get_str(qdict, "name");
>> bool new_state = qdict_get_bool(qdict, "option");
>> -    int ret = trace_event_set_state(tp_name, new_state);
>> 
>> -    if (!ret) {
>> -        monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
>> +    if (trace_event_is_pattern(tp_name)) {
>> +        TraceEvent *ev = NULL;
>> +        while ((ev = trace_event_pattern(tp_name, ev)) != NULL) {
>> +            if (!trace_event_get_state_static(ev)) {
>> +                monitor_printf(mon, "event \"%s\" is not traceable\n", tp_name);
>> +            }
>> +            trace_event_set_state_dynamic(ev, new_state);
>> +        }
>> +    } else {
>> +        TraceEvent *ev = trace_event_name(tp_name);
>> +        if (ev == NULL) {
>> +            monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
>> +        } else if (!trace_event_get_state_static(ev)) {
>> +            monitor_printf(mon, "event \"%s\" is not traceable\n", tp_name);
>> +        } else {
>> +            trace_event_set_state_dynamic(ev, new_state);
>> +        }

> Do we need to duplicate the pattern vs not-a-pattern case?

> We can loop with trace_event_pattern() and print the "unknown event
> name" only if !trace_event_is_pattern().

You mean something like this?


#v+
    bool found = false;
    TraceEvent *ev = NULL;
    while ((ev = trace_event_pattern(tp_name, ev)) != NULL) {
        found = true;
        if (!trace_event_get_state_static(ev)) {
            monitor_printf(mon, "event \"%s\" is not traceable\n", tp_name);
        } else {
            trace_event_set_state_dynamic(ev, new_state);
        }
    }
    if (!trace_event_is_pattern(tp_name) && !found) {
        monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
    }
#v-


Lluis
Stefan Hajnoczi Jan. 24, 2013, 9:37 a.m. UTC | #3 
On Wed, Jan 23, 2013 at 06:01:29PM +0100, Lluís Vilanova wrote:
> Stefan Hajnoczi writes:
> 
> > On Thu, Jan 10, 2013 at 08:23:19PM +0100, Lluís Vilanova wrote:
> >> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> >> ---
> >> monitor.c |   20 +++++++++++++++++---
> >> 1 file changed, 17 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/monitor.c b/monitor.c
> >> index 9cf419b..4c40541 100644
> >> --- a/monitor.c
> >> +++ b/monitor.c
> >> @@ -735,10 +735,24 @@ static void do_trace_event_set_state(Monitor *mon, const QDict *qdict)
> >> {
> >> const char *tp_name = qdict_get_str(qdict, "name");
> >> bool new_state = qdict_get_bool(qdict, "option");
> >> -    int ret = trace_event_set_state(tp_name, new_state);
> >> 
> >> -    if (!ret) {
> >> -        monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
> >> +    if (trace_event_is_pattern(tp_name)) {
> >> +        TraceEvent *ev = NULL;
> >> +        while ((ev = trace_event_pattern(tp_name, ev)) != NULL) {
> >> +            if (!trace_event_get_state_static(ev)) {
> >> +                monitor_printf(mon, "event \"%s\" is not traceable\n", tp_name);
> >> +            }
> >> +            trace_event_set_state_dynamic(ev, new_state);
> >> +        }
> >> +    } else {
> >> +        TraceEvent *ev = trace_event_name(tp_name);
> >> +        if (ev == NULL) {
> >> +            monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
> >> +        } else if (!trace_event_get_state_static(ev)) {
> >> +            monitor_printf(mon, "event \"%s\" is not traceable\n", tp_name);
> >> +        } else {
> >> +            trace_event_set_state_dynamic(ev, new_state);
> >> +        }
> 
> > Do we need to duplicate the pattern vs not-a-pattern case?
> 
> > We can loop with trace_event_pattern() and print the "unknown event
> > name" only if !trace_event_is_pattern().
> 
> You mean something like this?
> 
> 
> #v+
>     bool found = false;
>     TraceEvent *ev = NULL;
>     while ((ev = trace_event_pattern(tp_name, ev)) != NULL) {
>         found = true;
>         if (!trace_event_get_state_static(ev)) {
>             monitor_printf(mon, "event \"%s\" is not traceable\n", tp_name);
>         } else {
>             trace_event_set_state_dynamic(ev, new_state);
>         }
>     }
>     if (!trace_event_is_pattern(tp_name) && !found) {
>         monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
>     }
> #v-

Yes, exactly.  It's nice to have a single code path.

Stefan
diff mbox

Patch

diff --git a/monitor.c b/monitor.c
index 9cf419b..4c40541 100644
--- a/monitor.c
+++ b/monitor.c
@@ -735,10 +735,24 @@  static void do_trace_event_set_state(Monitor *mon, const QDict *qdict)
 {
     const char *tp_name = qdict_get_str(qdict, "name");
     bool new_state = qdict_get_bool(qdict, "option");
-    int ret = trace_event_set_state(tp_name, new_state);
 
-    if (!ret) {
-        monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
+    if (trace_event_is_pattern(tp_name)) {
+        TraceEvent *ev = NULL;
+        while ((ev = trace_event_pattern(tp_name, ev)) != NULL) {
+            if (!trace_event_get_state_static(ev)) {
+                monitor_printf(mon, "event \"%s\" is not traceable\n", tp_name);
+            }
+            trace_event_set_state_dynamic(ev, new_state);
+        }
+    } else {
+        TraceEvent *ev = trace_event_name(tp_name);
+        if (ev == NULL) {
+            monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
+        } else if (!trace_event_get_state_static(ev)) {
+            monitor_printf(mon, "event \"%s\" is not traceable\n", tp_name);
+        } else {
+            trace_event_set_state_dynamic(ev, new_state);
+        }
     }
 }