-msg parameter "timestamp" defaults to "off" if you don't specify msg,
and to "on" if you do.  Messed up right in commit 5e2ac51917 "add
timestamp to error_report()".  Mostly harmless, because "timestamp" is
the only parameter, so "if you do" is "-msg ''", which nobody does.

Change the default to "off" no matter what.

While there, rename enable_timestamp_msg to error_with_timestamp, and
polish documentation.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
I stumbled over this during review of Mario's "[PATCHv2 0/2] log guest
name and memory error type AO, AR for MCEs", which see.

 include/qemu/error-report.h |  3 ++-
 util/qemu-error.c           |  6 ++++--
 vl.c                        |  2 +-
 qemu-options.hx             | 10 +++++++---
 4 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
index 00d069b20f..87532d8596 100644
--- a/include/qemu/error-report.h
+++ b/include/qemu/error-report.h
@@ -73,6 +73,7 @@ void error_init(const char *argv0);
     })
 
 const char *error_get_progname(void);
-extern bool enable_timestamp_msg;
+
+extern bool error_with_timestamp;
 
 #endif
diff --git a/util/qemu-error.c b/util/qemu-error.c
index f373f3b3b0..dac7c7dc50 100644
--- a/util/qemu-error.c
+++ b/util/qemu-error.c
@@ -24,6 +24,9 @@ typedef enum {
     REPORT_TYPE_INFO,
 } report_type;
 
+/* Prepend timestamp to messages */
+bool error_with_timestamp;
+
 int error_printf(const char *fmt, ...)
 {
     va_list ap;
@@ -191,7 +194,6 @@ static void print_loc(void)
     }
 }
 
-bool enable_timestamp_msg;
 /*
  * Print a message to current monitor if we have one, else to stderr.
  * @report_type is the type of message: error, warning or informational.
@@ -204,7 +206,7 @@ static void vreport(report_type type, const char *fmt, va_list ap)
     GTimeVal tv;
     gchar *timestr;
 
-    if (enable_timestamp_msg && !cur_mon) {
+    if (error_with_timestamp && !cur_mon) {
         g_get_current_time(&tv);
         timestr = g_time_val_to_iso8601(&tv);
         error_printf("%s ", timestr);
diff --git a/vl.c b/vl.c
index 002bf4919e..b8e4c11f02 100644
--- a/vl.c
+++ b/vl.c
@@ -1262,7 +1262,7 @@ static void realtime_init(void)
 
 static void configure_msg(QemuOpts *opts)
 {
-    enable_timestamp_msg = qemu_opt_get_bool(opts, "timestamp", true);
+    error_with_timestamp = qemu_opt_get_bool(opts, "timestamp", false);
 }
 
 
diff --git a/qemu-options.hx b/qemu-options.hx
index 2a04ca6ac5..158244da0f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4165,13 +4165,17 @@ DEF("no-kvm", 0, QEMU_OPTION_no_kvm, "", QEMU_ARCH_I386)
 
 DEF("msg", HAS_ARG, QEMU_OPTION_msg,
     "-msg timestamp[=on|off]\n"
-    "                change the format of messages\n"
-    "                on|off controls leading timestamps (default:on)\n",
+    "                control error message format\n"
+    "                timestamp=on enables timestamps (default: off)\n",
     QEMU_ARCH_ALL)
 STEXI
 @item -msg timestamp[=on|off]
 @findex -msg
-prepend a timestamp to each log message.(default:on)
+Control error message format.
+@table @option
+@item timestamp=on|off
+Prefix messages with a timestamp.  Default is off.
+@end table
 ETEXI
 
 DEF("dump-vmstate", HAS_ARG, QEMU_OPTION_dump_vmstate,
-- 
2.21.0

Markus Armbruster <armbru@redhat.com> writes:

> -msg parameter "timestamp" defaults to "off" if you don't specify msg,
> and to "on" if you do.  Messed up right in commit 5e2ac51917 "add
> timestamp to error_report()".  Mostly harmless, because "timestamp" is
> the only parameter, so "if you do" is "-msg ''", which nobody does.
>
> Change the default to "off" no matter what.
>
> While there, rename enable_timestamp_msg to error_with_timestamp, and
> polish documentation.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Queued for 5.0.

I'll queue this patch.

On 10/10/19 10:15, Markus Armbruster wrote:
> -bool enable_timestamp_msg;

Would you like me to also rename his enable_guestname_msg variable to
error_with_guest_name?

Paolo

Paolo Bonzini <pbonzini@redhat.com> writes:

> I'll queue this patch.

Thanks!

> On 10/10/19 10:15, Markus Armbruster wrote:
>> -bool enable_timestamp_msg;
>
> Would you like me to also rename his enable_guestname_msg variable to
> error_with_guest_name?

In my review, I proposed replacing it by error_guest_name, to avoid
having to get -name parameter "guest" all the time.  I included a
revised patch to illustrate.  Goes on top of this one.  My revised patch
doesn't fully address my review.  In particular, there's a TODO in the
commit message.  Mario, would you be willing to complete it and post it?

On 21/10/19 09:03, Markus Armbruster wrote:
> In my review, I proposed replacing it by error_guest_name, to avoid
> having to get -name parameter "guest" all the time.  I included a
> revised patch to illustrate.  Goes on top of this one.  My revised patch
> doesn't fully address my review.  In particular, there's a TODO in the
> commit message.  Mario, would you be willing to complete it and post it?

Yes, it's better if I leave the whole series to you and Mario.  I'll
just get the KVM MCE changes.

Paolo