[Tracing,RFC] QMP interface to toggle state of a trace-event

QMP command trace-event to toggle state of a trace-event.
 Illustration :
 -> { "execute": "trace-event", "arguments": { "name": "qemu_malloc", "option": true} }
 <- { "return": {} }

Posting this as an RFC for now. I'll post the final version as a part of
 the cumulative QMP patchset for tracing ( including patches for query-* 
commands posted earlier : 
http://lists.gnu.org/archive/html/qemu-devel/2010-10/msg01232.html )

Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
---
 hmp-commands.hx |    2 +-
 monitor.c       |   43 +++++++++++++++++++++++++++++++++++++------
 qmp-commands.hx |   32 ++++++++++++++++++++++++++++++++
 3 files changed, 70 insertions(+), 7 deletions(-)

On Wed, 20 Oct 2010 15:28:49 +0530
Prerna Saxena <prerna@linux.vnet.ibm.com> wrote:

> QMP command trace-event to toggle state of a trace-event.
>  Illustration :
>  -> { "execute": "trace-event", "arguments": { "name": "qemu_malloc", "option": true} }
>  <- { "return": {} }
> 
> Posting this as an RFC for now. I'll post the final version as a part of
>  the cumulative QMP patchset for tracing ( including patches for query-* 
> commands posted earlier : 
> http://lists.gnu.org/archive/html/qemu-devel/2010-10/msg01232.html )
> 
> Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
> ---
>  hmp-commands.hx |    2 +-
>  monitor.c       |   43 +++++++++++++++++++++++++++++++++++++------
>  qmp-commands.hx |   32 ++++++++++++++++++++++++++++++++
>  3 files changed, 70 insertions(+), 7 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 81999aa..76ec2fe 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -149,7 +149,7 @@ ETEXI
>          .args_type  = "name:s,option:b",
>          .params     = "name on|off",
>          .help       = "changes status of a specific trace event",
> -        .mhandler.cmd = do_change_trace_event_state,
> +        .mhandler.cmd = do_change_trace_event_state_hmp,
>      },
>  
>  STEXI
> diff --git a/monitor.c b/monitor.c
> index c7e1f53..0766ed3 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -545,17 +545,43 @@ static void do_help_cmd(Monitor *mon, const QDict *qdict)
>  }
>  
>  #ifdef CONFIG_SIMPLE_TRACE
> -static void do_change_trace_event_state(Monitor *mon, const QDict *qdict)
> +
> +/**
> + * HMP handler to change trace event state.
> + *
> + */
> +void do_change_trace_event_state_hmp(Monitor *mon, const QDict *qdict)
>  {
> -    const char *tp_name = qdict_get_str(qdict, "name");
> -    bool new_state = qdict_get_bool(qdict, "option");
> -    int ret = st_change_trace_event_state(tp_name, new_state);
> +    if (!do_change_trace_event_state_generic(qdict)) {
> +        monitor_printf(mon, "unknown event name \"%s\"\n",
> +                                      qdict_get_str(qdict, "name"));
> +    }
> +}
>  
> -    if (!ret) {
> -        monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
> +/**
> + * QMP handler to change trace event state.
> + *
> + */
> +static int do_change_trace_event_state_qmp(Monitor *mon, const QDict *qdict,
> +                                           QObject **ret_data)
> +{
> +    if (!do_change_trace_event_state_generic(qdict)) {
> +        qerror_report(QERR_INVALID_PARAMETER, qdict_get_str(qdict, "name"));
> +        return -1;
>      }
> +    return 0;
>  }
>  
> +/**
> + * Generic handler to change trace event state.
> + *
> + */
> +static int do_change_trace_event_state_generic(const QDict *qdict)
> +{
> +    const char *tp_name = qdict_get_str(qdict, "name");
> +    bool new_state = qdict_get_bool(qdict, "option");
> +    return st_change_trace_event_state(tp_name, new_state);
> +}
>  static void do_trace_file(Monitor *mon, const QDict *qdict)
>  {
>      const char *op = qdict_get_try_str(qdict, "op");
> @@ -583,6 +609,11 @@ static void do_info_trace_file_to_qmp(Monitor *mon, QObject **ret_data)
>  {
>      *ret_data = st_print_file_to_qobject();
>  }
> +
> +#else
> +static int do_change_trace_event_state_qmp(Monitor *mon, const QDict *qdict,
> +QObject **ret_data) {}
> +
>  #endif
>  
>  static void user_monitor_complete(void *opaque, QObject *ret_data)
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index bc79b55..7613d73 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -761,6 +761,38 @@ Example:
>  
>  Note: This command must be issued before issuing any other command.
>  
> +EQMP
> +
> +    {
> +        .name       = "trace-event",
> +        .args_type  = "name:s,option:b",
> +        .params     = "name on|off",
> +        .help       = "changes state of a specific trace event",
> +        .user_print = monitor_user_noop,
> +        .mhandler.cmd_new = do_change_trace_event_state_qmp,
> +    },
> +
> +SQMP
> +trace-event
> +-----------
> +
> +Change state of a trace-event.
> +
> +Arguments:
> +
> +- "name": name of trace-event (json-string)
> +- "option": new state for the trace-event (json-bool)

This should be called 'enabled'.

I think you should submit a new series containing only the proposed
interfaces documentation (one patch per interface) and the intro email
should describe the use cases the proposed interfaces are supposed to
address.

> +
> +Example:
> +
> +-> { "execute": "trace-event", "arguments": { "name": "ABC", "option":false } }
> +<- { "return": {} }
> +
> +Notes:
> +
> +(1) The 'query-trace-events' command should be used to check the new state 
> +    of the trace-event.
> +
>  3. Query Commands
>  =================
>

Thanks for the review!

On 10/21/2010 12:53 AM, Luiz Capitulino wrote:
> On Wed, 20 Oct 2010 15:28:49 +0530
> Prerna Saxena<prerna@linux.vnet.ibm.com>  wrote:
>
>> QMP command trace-event to toggle state of a trace-event.
>>   Illustration :
>>   ->  { "execute": "trace-event", "arguments": { "name": "qemu_malloc", "option": true} }
>>   <- { "return": {} }
>>
>> Posting this as an RFC for now. I'll post the final version as a part of
>>   the cumulative QMP patchset for tracing ( including patches for query-*
>> commands posted earlier :
>> http://lists.gnu.org/archive/html/qemu-devel/2010-10/msg01232.html )
>>
>> Signed-off-by: Prerna Saxena<prerna@linux.vnet.ibm.com>
>> ---
>>   hmp-commands.hx |    2 +-
>>   monitor.c       |   43 +++++++++++++++++++++++++++++++++++++------
>>   qmp-commands.hx |   32 ++++++++++++++++++++++++++++++++
>>   3 files changed, 70 insertions(+), 7 deletions(-)
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index bc79b55..7613d73 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -761,6 +761,38 @@ Example:
>>
>>   Note: This command must be issued before issuing any other command.
>>
>> +EQMP
>> +
>> +    {
>> +        .name       = "trace-event",
>> +        .args_type  = "name:s,option:b",
>> +        .params     = "name on|off",
>> +        .help       = "changes state of a specific trace event",
>> +        .user_print = monitor_user_noop,
>> +        .mhandler.cmd_new = do_change_trace_event_state_qmp,
>> +    },
>> +
>> +SQMP
>> +trace-event
>> +-----------
>> +
>> +Change state of a trace-event.
>> +
>> +Arguments:
>> +
>> +- "name": name of trace-event (json-string)
>> +- "option": new state for the trace-event (json-bool)
>
> This should be called 'enabled'.
>

I agree, 'enabled' is less ambiguous. Will change in the next patchset.


> I think you should submit a new series containing only the proposed
> interfaces documentation (one patch per interface) and the intro email
> should describe the use cases the proposed interfaces are supposed to
> address.

I'll send out the new documentation patchset series shortly.

>> +
>> +Example:
>> +
>> +->  { "execute": "trace-event", "arguments": { "name": "ABC", "option":false } }
>> +<- { "return": {} }
>> +
>> +Notes:
>> +
>> +(1) The 'query-trace-events' command should be used to check the new state
>> +    of the trace-event.
>> +
>>   3. Query Commands
>>   =================
>>
>