Message ID | 1445850841-29510-2-git-send-email-den@openvz.org |
---|---|
State | New |
Headers | show |
Denis V Lunev writes: > From: Paolo Bonzini <pbonzini@redhat.com> > This is a bit easier to use than "-trace" if you are also enabling > other kinds of logging. It is also more discoverable for experienced > QEMU users, and accessible from user-mode emulators. I'm not sure this should be added, since the same functionality is also available through "-trace enable=<pattern>" (and the shortcut "-trace <pattern>"). Also, I'd rather fold event name discovery into "-trace enable=?" (and the shortcut "-trace ?"), mimicking the format already available for CPUs ("-cpu ?"). Cheers, Lluis
On 10/26/2015 07:12 AM, Lluís Vilanova wrote: > Denis V Lunev writes: > >> From: Paolo Bonzini <pbonzini@redhat.com> >> This is a bit easier to use than "-trace" if you are also enabling >> other kinds of logging. It is also more discoverable for experienced >> QEMU users, and accessible from user-mode emulators. > > I'm not sure this should be added, since the same functionality is also > available through "-trace enable=<pattern>" (and the shortcut "-trace > <pattern>"). Having more than one way to do something is not necessarily bad; it does imply more maintenance to keep both ways working, but if one way is more discoverable than the other it may be worth it. > > Also, I'd rather fold event name discovery into "-trace enable=?" (and the > shortcut "-trace ?"), mimicking the format already available for CPUs ("-cpu > ?"). > If we do that, we should also support '-trace enable=help', because ? is a shell metacharacter, and we have been moving towards using 'help' rather than '?' to minimize the need for shell quoting when asking for help.
Eric Blake writes: > On 10/26/2015 07:12 AM, Lluís Vilanova wrote: >> Denis V Lunev writes: >> >>> From: Paolo Bonzini <pbonzini@redhat.com> >>> This is a bit easier to use than "-trace" if you are also enabling >>> other kinds of logging. It is also more discoverable for experienced >>> QEMU users, and accessible from user-mode emulators. >> >> I'm not sure this should be added, since the same functionality is also >> available through "-trace enable=<pattern>" (and the shortcut "-trace >> <pattern>"). > Having more than one way to do something is not necessarily bad; it does > imply more maintenance to keep both ways working, but if one way is more > discoverable than the other it may be worth it. Certainly true. I just find it confusing to have the same functionality available through different forms. >> >> Also, I'd rather fold event name discovery into "-trace enable=?" (and the >> shortcut "-trace ?"), mimicking the format already available for CPUs ("-cpu >> ?"). >> > If we do that, we should also support '-trace enable=help', because ? is > a shell metacharacter, and we have been moving towards using 'help' > rather than '?' to minimize the need for shell quoting when asking for help. Oh, I wasn't aware of the "deprecation" of '?'. Then it certainly makes more sense to use 'help'. Thanks, Lluis
diff --git a/util/log.c b/util/log.c index 5c641a0..2bcef95 100644 --- a/util/log.c +++ b/util/log.c @@ -19,6 +19,7 @@ #include "qemu-common.h" #include "qemu/log.h" +#include "trace/control.h" static char *logfilename; FILE *qemu_logfile; @@ -154,6 +155,11 @@ int qemu_str_to_log_mask(const char *str) for (item = qemu_log_items; item->mask != 0; item++) { mask |= item->mask; } +#ifdef CONFIG_TRACE_LOG + } else if (strncmp(p, "trace:", 6) == 0 && p + 6 != p1) { + trace_enable_events(p + 6); + mask |= LOG_TRACE; +#endif } else { for (item = qemu_log_items; item->mask != 0; item++) { if (cmp1(p, p1 - p, item->name)) { @@ -161,9 +167,9 @@ int qemu_str_to_log_mask(const char *str) } } return 0; + found: + mask |= item->mask; } - found: - mask |= item->mask; if (*p1 != ',') { break; } @@ -177,6 +183,10 @@ void qemu_print_log_usage(FILE *f) const QEMULogItem *item; fprintf(f, "Log items (comma separated):\n"); for (item = qemu_log_items; item->mask != 0; item++) { - fprintf(f, "%-10s %s\n", item->name, item->help); + fprintf(f, "%-15s %s\n", item->name, item->help); } +#ifdef CONFIG_TRACE_LOG + fprintf(f, "trace:PATTERN enable trace events\n"); + fprintf(f, "\nUse \"-d trace:help\" to get a list of trace events.\n\n"); +#endif }