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

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

[Dongli Zhang]

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, the options of "trace:PATTERN" are only printed by
"qemu-system-x86_64 -d help", but missing in hmp "help log" command.

Fixes: c84ea00dc2 ("log: add "-d trace:PATTERN"")
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.
Changed since v2:
- use "log trace:help" in help message.
- add more clarification in commit message.
- add 'Fixes' tag.
---
 monitor/hmp.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/monitor/hmp.c b/monitor/hmp.c
index 15ca04735c..a3375d0341 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 \"log trace:help\" to get a list of "
+                           "trace events.\n\n");
+#endif
             return;
         }
 
-- 
2.17.1

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

[Markus Armbruster]

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, the options of "trace:PATTERN" are only printed by
> "qemu-system-x86_64 -d help", but missing in hmp "help log" command.
>
> Fixes: c84ea00dc2 ("log: add "-d trace:PATTERN"")
> 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.
> Changed since v2:
> - use "log trace:help" in help message.
> - add more clarification in commit message.
> - add 'Fixes' tag.
> ---
>  monitor/hmp.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> index 15ca04735c..a3375d0341 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 \"log trace:help\" to get a list of "
> +                           "trace events.\n\n");
> +#endif
>              return;
>          }

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Not this patch's fault:

1. "-d help" terminates with exit status 1, "-d trace:help" with 0.  The
   former is wrong.

2. HMP "log trace:help" prints to stdout instead of the current monitor.

3. Output of HMP "log trace:help" sometimes is truncated for me.

4. Output of "log trace:help" and "info trace-events" is unwieldy.
   Sorted output could be a bit less unwieldy.

5. Could "log trace:help" and "info trace-events" share code?

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

[Philippe Mathieu-Daudé via]

Hi Markus,

On 2/9/22 14:24, 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, the options of "trace:PATTERN" are only printed by
>> "qemu-system-x86_64 -d help", but missing in hmp "help log" command.
>>
>> Fixes: c84ea00dc2 ("log: add "-d trace:PATTERN"")
>> 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.
>> Changed since v2:
>> - use "log trace:help" in help message.
>> - add more clarification in commit message.
>> - add 'Fixes' tag.
>> ---
>>   monitor/hmp.c | 9 +++++++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)

> Not this patch's fault:
> 
> 1. "-d help" terminates with exit status 1, "-d trace:help" with 0.  The
>     former is wrong.
> 
> 2. HMP "log trace:help" prints to stdout instead of the current monitor.
> 
> 3. Output of HMP "log trace:help" sometimes is truncated for me.
> 
> 4. Output of "log trace:help" and "info trace-events" is unwieldy.
>     Sorted output could be a bit less unwieldy.
> 
> 5. Could "log trace:help" and "info trace-events" share code?

Do you mind opening issue(s) on our GitLab so we don't loose your
analysis buried within the infinite mailing list?

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

[Dongli Zhang]

Hi Markus,

On 9/17/22 2:44 PM, Philippe Mathieu-Daudé via wrote:
> Hi Markus,
> 
> On 2/9/22 14:24, 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, the options of "trace:PATTERN" are only printed by
>>> "qemu-system-x86_64 -d help", but missing in hmp "help log" command.
>>>
>>> Fixes: c84ea00dc2 ("log: add "-d trace:PATTERN"")
>>> 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.
>>> Changed since v2:
>>> - use "log trace:help" in help message.
>>> - add more clarification in commit message.
>>> - add 'Fixes' tag.
>>> ---
>>>   monitor/hmp.c | 9 +++++++--
>>>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
>> Not this patch's fault:
>>
>> 1. "-d help" terminates with exit status 1, "-d trace:help" with 0.  The
>>     former is wrong.

May I assume it is expected to have exit status 1 when "-d help"?

According to the output of "-d", there is even not a "help" option, but only a
"-d trace:help" option. That is, "-d help" is not officially supported.

The below example use "-d hellworld" but not "help".

# qemu-system-x86_64 -d helloworld
Log items (comma separated):
out_asm         show generated host assembly code for each compiled TB
in_asm          show target assembly code for each compiled TB
op              show micro ops for each compiled TB
op_opt          show micro ops after optimization
op_ind          show micro ops before indirect lowering
int             show interrupts/exceptions in short format
exec            show trace before each executed TB (lots of logs)
cpu             show CPU registers before entering a TB (lots of logs)
fpu             include FPU registers in the 'cpu' logging
mmu             log MMU-related activities
pcall           x86 only: show protected mode far calls/returns/exceptions
cpu_reset       show CPU state before CPU resets
unimp           log unimplemented functionality
guest_errors    log when the guest OS does something invalid (eg accessing a
non-existent register)
page            dump pages at beginning of user mode emulation
nochain         do not chain compiled TBs so that "exec" and "cpu" show
complete traces
plugin          output from TCG plugins

strace          log every user-mode syscall, its input, and its result
tid             open a separate log file per thread; filename must contain '%d'
trace:PATTERN   enable trace events

Use "-d trace:help" to get a list of trace events.


According to the source code, the qemu_str_to_log_mask() expects either log
items or "trace". For any other inputs (e.g., "help" or "helloworld"),
qemu_str_to_log_mask() returns 0 (no bit set in the mask). That indicates the
input (e.g., "help") is not an expected input.

Therefore, can I assume this is not a bug? I do not think something like below
is very helpful.

diff --git a/softmmu/vl.c b/softmmu/vl.c
index 263f029a8e..54c8e624bf 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2389,6 +2389,8 @@ static void qemu_process_early_options(void)
             mask = qemu_str_to_log_mask(log_mask);
             if (!mask) {
                 qemu_print_log_usage(stdout);
+                if (g_str_equal(log_mask, "help"))
+                    exit(0)
                 exit(1);
             }
         }

Thank you very much!

Dongli Zhang


>>
>> 2. HMP "log trace:help" prints to stdout instead of the current monitor.
>>
>> 3. Output of HMP "log trace:help" sometimes is truncated for me.
>>
>> 4. Output of "log trace:help" and "info trace-events" is unwieldy.
>>     Sorted output could be a bit less unwieldy.
>>
>> 5. Could "log trace:help" and "info trace-events" share code?
> 
> Do you mind opening issue(s) on our GitLab so we don't loose your
> analysis buried within the infinite mailing list?
> 

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

[Markus Armbruster]

Dongli Zhang <dongli.zhang@oracle.com> writes:

> Hi Markus,
>
> On 9/17/22 2:44 PM, Philippe Mathieu-Daudé via wrote:
>> Hi Markus,
>> 
>> On 2/9/22 14:24, 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, the options of "trace:PATTERN" are only printed by
>>>> "qemu-system-x86_64 -d help", but missing in hmp "help log" command.
>>>>
>>>> Fixes: c84ea00dc2 ("log: add "-d trace:PATTERN"")
>>>> 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.
>>>> Changed since v2:
>>>> - use "log trace:help" in help message.
>>>> - add more clarification in commit message.
>>>> - add 'Fixes' tag.
>>>> ---
>>>>   monitor/hmp.c | 9 +++++++--
>>>>   1 file changed, 7 insertions(+), 2 deletions(-)
>> 
>>> Not this patch's fault:
>>>
>>> 1. "-d help" terminates with exit status 1, "-d trace:help" with 0.  The
>>>     former is wrong.
>
> May I assume it is expected to have exit status 1 when "-d help"?

Non-zero exit status means error.  Asking for and receiving help is not
an error.  Therefore, "-d help" should exit with status 0, just like
"-help", "-device help", "-machine help", ...

> According to the output of "-d", there is even not a "help" option, but only a
> "-d trace:help" option. That is, "-d help" is not officially supported.

It *is* documented:

    $ qemu-system-x86_64 -help | grep -- '^-d '
    -d item1,...    enable logging of specified items (use '-d help' for a list of log items)

> The below example use "-d hellworld" but not "help".
>
> # qemu-system-x86_64 -d helloworld
> Log items (comma separated):
> out_asm         show generated host assembly code for each compiled TB
> in_asm          show target assembly code for each compiled TB
> op              show micro ops for each compiled TB
> op_opt          show micro ops after optimization
> op_ind          show micro ops before indirect lowering
> int             show interrupts/exceptions in short format
> exec            show trace before each executed TB (lots of logs)
> cpu             show CPU registers before entering a TB (lots of logs)
> fpu             include FPU registers in the 'cpu' logging
> mmu             log MMU-related activities
> pcall           x86 only: show protected mode far calls/returns/exceptions
> cpu_reset       show CPU state before CPU resets
> unimp           log unimplemented functionality
> guest_errors    log when the guest OS does something invalid (eg accessing a
> non-existent register)
> page            dump pages at beginning of user mode emulation
> nochain         do not chain compiled TBs so that "exec" and "cpu" show
> complete traces
> plugin          output from TCG plugins
>
> strace          log every user-mode syscall, its input, and its result
> tid             open a separate log file per thread; filename must contain '%d'
> trace:PATTERN   enable trace events
>
> Use "-d trace:help" to get a list of trace events.
>
>
> According to the source code, the qemu_str_to_log_mask() expects either log
> items or "trace". For any other inputs (e.g., "help" or "helloworld"),
> qemu_str_to_log_mask() returns 0 (no bit set in the mask).

You're right.

>                                                            That indicates the
> input (e.g., "help") is not an expected input.

No, it indicates laziness :)

> Therefore, can I assume this is not a bug? I do not think something like below
> is very helpful.
>
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 263f029a8e..54c8e624bf 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2389,6 +2389,8 @@ static void qemu_process_early_options(void)
>              mask = qemu_str_to_log_mask(log_mask);
>              if (!mask) {
>                  qemu_print_log_usage(stdout);
> +                if (g_str_equal(log_mask, "help"))
> +                    exit(0)
>                  exit(1);
>              }
>          }

Let's make "-d help" print help to stdout and terminate successfully,
and "-d crap" report an error and terminate unsuccessfully.  Just like
other options, such as -device and -machine.

> Thank you very much!

You're welcome!

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

[Dr. David Alan Gilbert]

* Dongli Zhang (dongli.zhang@oracle.com) wrote:
> 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, the options of "trace:PATTERN" are only printed by
> "qemu-system-x86_64 -d help", but missing in hmp "help log" command.
> 
> Fixes: c84ea00dc2 ("log: add "-d trace:PATTERN"")
> Cc: Joe Jin <joe.jin@oracle.com>
> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>

Queued

> ---
> Changed since v1:
> - change format for "none" as well.
> Changed since v2:
> - use "log trace:help" in help message.
> - add more clarification in commit message.
> - add 'Fixes' tag.
> ---
>  monitor/hmp.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> index 15ca04735c..a3375d0341 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 \"log trace:help\" to get a list of "
> +                           "trace events.\n\n");
> +#endif
>              return;
>          }
>  
> -- 
> 2.17.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK