Re: [PATCH v2 1/1] monitor/hmp: print trace as option in help for log command

[Dongli Zhang]

Hi Markus,

On 8/30/22 4:04 AM, Markus Armbruster wrote:
> Dongli Zhang <dongli.zhang@oracle.com> writes:
> 
>> The below is printed when printing help information in qemu-system-x86_64
>> command line, and when CONFIG_TRACE_LOG is enabled:
>>
>> $ qemu-system-x86_64 -d help
>> ... ...
>> trace:PATTERN   enable trace events
>>
>> Use "-d trace:help" to get a list of trace events.
>>
>> However, they are not printed in hmp "help log" command.
> 
> This leaves me guessing what exactly the patch tries to do.

I will clarify in the commit message.

> 
>> Cc: Joe Jin <joe.jin@oracle.com>
>> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
>> ---
>> Changed since v1:
>> - change format for "none" as well.
>>
>>  monitor/hmp.c | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/monitor/hmp.c b/monitor/hmp.c
>> index 15ca047..467fc84 100644
>> --- a/monitor/hmp.c
>> +++ b/monitor/hmp.c
>> @@ -285,10 +285,15 @@ void help_cmd(Monitor *mon, const char *name)
>>          if (!strcmp(name, "log")) {
>>              const QEMULogItem *item;
>>              monitor_printf(mon, "Log items (comma separated):\n");
>> -            monitor_printf(mon, "%-10s %s\n", "none", "remove all logs");
>> +            monitor_printf(mon, "%-15s %s\n", "none", "remove all logs");
>>              for (item = qemu_log_items; item->mask != 0; item++) {
>> -                monitor_printf(mon, "%-10s %s\n", item->name, item->help);
>> +                monitor_printf(mon, "%-15s %s\n", item->name, item->help);
>>              }
>> +#ifdef CONFIG_TRACE_LOG
>> +            monitor_printf(mon, "trace:PATTERN   enable trace events\n");
>> +            monitor_printf(mon, "\nUse \"info trace-events\" to get a list 
>> of "
>> +                                "trace events.\n\n");
> 
> Aha: it fixes help to show "log trace:PATTERN".  Was that forgotten in
> Paolo's commit c84ea00dc2 'log: add "-d trace:PATTERN"'?

I will add the Fixes tag.

> 
> "info trace-events", hmmm... it shows trace events and their state.
> "log trace:help" also lists them, less their state, and in opposite
> order.  Why do we need both?

I will print "log trace:help" in the help output.

> 
> What about showing them in alphabetical order?

The order is following how they are defined in the qemu_log_items[] array. To
re-order them in the array may introduce more conflicts when backporting a
util/log patch to QEMU old version.

Please let me know if you prefer to re-order. Otherwise, I prefer to avoid that.

Thank you very much for the suggestions!

Dongli Zhang

> 
>> +#endif
>>              return;
>>          }
>