Hi Markus,
On 8/31/22 11:47 PM, Markus Armbruster wrote:
> Dongli Zhang <dongli.zh...@oracle.com> writes:
>
>> Hi Markus,
>>
>> On 8/30/22 4:04 AM, Markus Armbruster wrote:
>>> Dongli Zhang <dongli.zh...@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....@oracle.com>
>>>> Signed-off-by: Dongli Zhang <dongli.zh...@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 guess we have both because we want an HMP command to show the state of
> trace events ("info trace-events"), and we want "-d trace" to provide
> help.
>
> The latter also lets HMP command "log trace" help, which feels less
> important to me, since "info trace-events" exists and is easier to find
> and significantly more usable than "log trace:help": it can filter its
> output, and unfiltered output is too long to be useful without something
> like grep.
>
> Could the two share more code?
Thank you very much for the suggestion. I will try if they can share the code.
>
> Hmm, there seems to be something wrong with "log trace:help": I see
> truncated output. Moreover, output goes to stdout instead of the
> monitor. That's wrong. Any help you can also emit from the monitor
> should be printed with qemu_printf().
Currently the "log trace:help" prints via fprintf() to stdout. I will fix this.
169 void trace_list_events(FILE *f)
170 {
171 TraceEventIter iter;
172 TraceEvent *ev;
173 trace_event_iter_init_all(&iter);
174 while ((ev = trace_event_iter_next(&iter)) != NULL) {
175 fprintf(f, "%s\n", trace_event_get_name(ev));
176 }
177 #ifdef CONFIG_TRACE_DTRACE
178 fprintf(f, "This list of names of trace points may be incomplete "
179 "when using the DTrace/SystemTap backends.\n"
180 "Run 'qemu-trace-stap list %s' to print the full list.\n",
181 g_get_prgname());
182 #endif
183 }
>
>> 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.
>
> I'm talking about the output of "log trace:help", not the output of "log
> help".
The order is guaranteed by each trace group. To re-order may require some works
with scripts/tracetool when the events from each group/folder are united. I will
have a confirm.
Thank you very much for suggestions!
Dongli Zhang
