Re: [PATCH 2/4] virsh: Add timestamps to events

[Andrea Bolognani <abologna@xxxxxxxxxx>]

On Mon, 2015-12-21 at 11:04 +0100, Jiri Denemark wrote:
> 
> @@ -12154,7 +12155,16 @@ virshEventPrint(virshDomEventData *data,
>      if (!data->loop && *data->count)
>          goto cleanup;
>  
> -    vshPrint(data->ctl, "%s", msg);
> +    if (data->timestamp) {
> +        char timestamp[VIR_TIME_STRING_BUFLEN];
> +
> +        if (virTimeStringNowRaw(timestamp) < 0)
> +            timestamp[0] = '\0';
> +
> +        vshPrint(data->ctl, "%s: %s", timestamp, msg);
> +    } else {
> +        vshPrint(data->ctl, "%s", msg);
> +    }

So the timestamp is not really for the moment the even occurred,
rather the moment it was printed. I don't know if the difference is
something we should really care about.

The signature for virConnectDomainEventGenericCallback, unlike the one
for virConnectDomainQemuMonitorEventCallback, does not provide timining
information. Would it be a good idea to maybe have a new kind of
callback for generic events that includes such information, instead of
using the timestamp from when we printed the message?

What about having qemu-monitor-event support --timestamp as well? I know
the timing information are already part of the output, but we could use
the same format used here (much more readable) and avoid duplicating
them in the message. POC attached.

Just bouncing back my ideas. You probably have very good reasons to be
doing it this way :)

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

From bdd767a490031981b926c2db537d2278c5732e18 Mon Sep 17 00:00:00 2001
From: Andrea Bolognani <abologna@xxxxxxxxxx>
Date: Mon, 21 Dec 2015 16:22:58 +0100
Subject: [PATCH] virsh: Add timestamps to QEMU monitor events

Implement a --timestamp option for qemu-monitor-event, similar to the
one for events.

If the option is present, the human-readable timestamp will be printed
before the message, and the message will contain no timing information.
---
 tools/virsh-domain.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 6e219de..850bc7b 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9157,6 +9157,7 @@ struct virshQemuEventData {
     vshControl *ctl;
     bool loop;
     bool pretty;
+    bool timestamp;
     int count;
 };
 typedef struct virshQemuEventData virshQemuEventData;
@@ -9181,8 +9182,20 @@ virshEventQemuPrint(virConnectPtr conn ATTRIBUTE_UNUSED,
         if (pretty && (str = virJSONValueToString(pretty, true)))
             details = str;
     }
-    vshPrint(data->ctl, "event %s at %lld.%06u for domain %s: %s\n",
-             event, seconds, micros, virDomainGetName(dom), NULLSTR(details));
+
+    if (data->timestamp) {
+        char timestamp[VIR_TIME_STRING_BUFLEN];
+
+        if (virTimeStringNowRaw(timestamp) < 0)
+            timestamp[0] = '\0';
+
+        vshPrint(data->ctl, "%s: event %s for domain %s: %s\n",
+                 timestamp, event, virDomainGetName(dom), NULLSTR(details));
+    } else {
+        vshPrint(data->ctl, "event %s at %lld.%06u for domain %s: %s\n",
+                 event, seconds, micros, virDomainGetName(dom), NULLSTR(details));
+    }
+
     data->count++;
     if (!data->loop)
         vshEventDone(data->ctl);
@@ -9229,6 +9242,10 @@ static const vshCmdOptDef opts_qemu_monitor_event[] = {
      .type = VSH_OT_BOOL,
      .help = N_("treat event case-insensitively")
     },
+    {.name = "timestamp",
+     .type = VSH_OT_BOOL,
+     .help = N_("show timestamp for each printed event")
+    },
     {.name = NULL}
 };
 
@@ -9252,6 +9269,7 @@ cmdQemuMonitorEvent(vshControl *ctl, const vshCmd *cmd)
     data.ctl = ctl;
     data.loop = vshCommandOptBool(cmd, "loop");
     data.pretty = vshCommandOptBool(cmd, "pretty");
+    data.timestamp = vshCommandOptBool(cmd, "timestamp");
     data.count = 0;
     if (vshCommandOptTimeoutToMs(ctl, cmd, &timeout) < 0)
         return false;
-- 
2.5.0

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list