[Qemu-devel] [PATCH v2] error: only prepend timestamp on stderr

[Qemu-devel] [PATCH v2] error: only prepend timestamp on stderr

[Stefan Hajnoczi]

The -msg timestamp=on option prepends a timestamp to error messages.
This is useful on stderr where it allows users to identify when an error
was raised.

Timestamps do not make sense on the monitor since error_report() is
called in response to a synchronous monitor command and the user already
knows "when" the command was issued.  Additionally, the rest of the
monitor conversation lacks timestamps so the error timestamp cannot be
correlated with other activity.

Only prepend timestamps on stderr.  This fixes libvirt's 'drive_del'
processing, which did not expect a timestamp.  Other QEMU monitor
clients are probably equally confused by timestamps on monitor error
messages.

Cc: Markus Armbruster <armbru@redhat.com>
Cc: Seiji Aguchi <seiji.aguchi@hds.com>
Cc: Frank Schreuder <fschreuder@transip.nl>
Cc: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/qemu-error.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/qemu-error.c b/util/qemu-error.c
index 77ea6c6..c1574bb 100644
--- a/util/qemu-error.c
+++ b/util/qemu-error.c
@@ -210,7 +210,7 @@ void error_vreport(const char *fmt, va_list ap)
     GTimeVal tv;
     gchar *timestr;
 
-    if (enable_timestamp_msg) {
+    if (enable_timestamp_msg && !cur_mon) {
         g_get_current_time(&tv);
         timestr = g_time_val_to_iso8601(&tv);
         error_printf("%s ", timestr);
-- 
2.4.3

Re: [Qemu-devel] [PATCH v2] error: only prepend timestamp on stderr

[Daniel P. Berrange]

On Mon, Aug 10, 2015 at 02:15:41PM +0100, Stefan Hajnoczi wrote:
> The -msg timestamp=on option prepends a timestamp to error messages.
> This is useful on stderr where it allows users to identify when an error
> was raised.
> 
> Timestamps do not make sense on the monitor since error_report() is
> called in response to a synchronous monitor command and the user already
> knows "when" the command was issued.  Additionally, the rest of the
> monitor conversation lacks timestamps so the error timestamp cannot be
> correlated with other activity.
> 
> Only prepend timestamps on stderr.  This fixes libvirt's 'drive_del'
> processing, which did not expect a timestamp.  Other QEMU monitor
> clients are probably equally confused by timestamps on monitor error
> messages.
> 
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Seiji Aguchi <seiji.aguchi@hds.com>
> Cc: Frank Schreuder <fschreuder@transip.nl>
> Cc: Daniel P. Berrange <berrange@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  util/qemu-error.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v2] error: only prepend timestamp on stderr

[Frank Schreuder]

Op 8/10/2015 om 3:15 PM schreef Stefan Hajnoczi:
> The -msg timestamp=on option prepends a timestamp to error messages.
> This is useful on stderr where it allows users to identify when an error
> was raised.
>
> Timestamps do not make sense on the monitor since error_report() is
> called in response to a synchronous monitor command and the user already
> knows "when" the command was issued.  Additionally, the rest of the
> monitor conversation lacks timestamps so the error timestamp cannot be
> correlated with other activity.
>
> Only prepend timestamps on stderr.  This fixes libvirt's 'drive_del'
> processing, which did not expect a timestamp.  Other QEMU monitor
> clients are probably equally confused by timestamps on monitor error
> messages.
>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Seiji Aguchi <seiji.aguchi@hds.com>
> Cc: Frank Schreuder <fschreuder@transip.nl>
> Cc: Daniel P. Berrange <berrange@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  util/qemu-error.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/util/qemu-error.c b/util/qemu-error.c
> index 77ea6c6..c1574bb 100644
> --- a/util/qemu-error.c
> +++ b/util/qemu-error.c
> @@ -210,7 +210,7 @@ void error_vreport(const char *fmt, va_list ap)
>      GTimeVal tv;
>      gchar *timestr;
>  
> -    if (enable_timestamp_msg) {
> +    if (enable_timestamp_msg && !cur_mon) {
>          g_get_current_time(&tv);
>          timestr = g_time_val_to_iso8601(&tv);
>          error_printf("%s ", timestr);
Tested-by: Frank Schreuder <fschreuder@transip.nl>

Regards,
Frank

Re: [Qemu-devel] [PATCH v2] error: only prepend timestamp on stderr

[Markus Armbruster]

Stefan Hajnoczi <stefanha@redhat.com> writes:

> The -msg timestamp=on option prepends a timestamp to error messages.
> This is useful on stderr where it allows users to identify when an error
> was raised.
>
> Timestamps do not make sense on the monitor since error_report() is
> called in response to a synchronous monitor command and the user already
> knows "when" the command was issued.  Additionally, the rest of the
> monitor conversation lacks timestamps so the error timestamp cannot be
> correlated with other activity.
>
> Only prepend timestamps on stderr.  This fixes libvirt's 'drive_del'
> processing, which did not expect a timestamp.  Other QEMU monitor
> clients are probably equally confused by timestamps on monitor error
> messages.
>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Seiji Aguchi <seiji.aguchi@hds.com>
> Cc: Frank Schreuder <fschreuder@transip.nl>
> Cc: Daniel P. Berrange <berrange@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

I'll take this through my tree.  Thanks!