| Message ID | 20220831213943.8155-1-dongli.zhang@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/1] monitor/hmp: print trace as option in help for log command | expand |
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?
* 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 >
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?
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? >
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!
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; }
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(-)