[PATCH 07/12] util: introduce common helper for error-report & log code

Daniel P. Berrangé via Devel posted 12 patches 2 weeks, 5 days ago
There is a newer version of this series
[PATCH 07/12] util: introduce common helper for error-report & log code
Posted by Daniel P. Berrangé 2 weeks, 5 days ago
The error-report and log code both have a need to add prefixes
to messages they are printing, with the current example being
a timestamp.

The format and configuration they use should be consistent, so
providing a common helper will ensure this is always the case.
Initially the helper only emits a timestamp, but future patches
will expand this.

This takes the liberty of assigning the new file to the same
maintainer as the existing error-report.c file, given it will
be extracting some functionality from the latter.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 MAINTAINERS            |  2 ++
 include/qemu/message.h | 40 ++++++++++++++++++++++++++++++++++++++++
 util/meson.build       |  1 +
 util/message.c         | 32 ++++++++++++++++++++++++++++++++
 4 files changed, 75 insertions(+)
 create mode 100644 include/qemu/message.h
 create mode 100644 util/message.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a07086ed76..3cc6c0b409 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3169,9 +3169,11 @@ M: Markus Armbruster <armbru@redhat.com>
 S: Supported
 F: include/qapi/error.h
 F: include/qemu/error-report.h
+F: include/qemu/message.h
 F: qapi/error.json
 F: util/error.c
 F: util/error-report.c
+F: util/message.c
 F: scripts/coccinelle/err-bad-newline.cocci
 F: scripts/coccinelle/error-use-after-free.cocci
 F: scripts/coccinelle/error_propagate_null.cocci
diff --git a/include/qemu/message.h b/include/qemu/message.h
new file mode 100644
index 0000000000..160bee8417
--- /dev/null
+++ b/include/qemu/message.h
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef QEMU_MESSAGE_H
+#define QEMU_MESSAGE_H
+
+enum QMessageFormatFlags {
+    QMESSAGE_FORMAT_TIMESTAMP = (1 << 0),
+};
+
+/*
+ * qmessage_set_format:
+ * @flags: the message information to emit
+ *
+ * Select which pieces of information to
+ * emit for messages
+ */
+void qmessage_set_format(int flags);
+
+enum QMessageContextFlags {
+    QMESSAGE_CONTEXT_SKIP_MONITOR = (1 << 0),
+};
+
+/*
+ * qmessage_context:
+ * @flags: the message formatting control flags
+ *
+ * Format a message prefix with the information
+ * previously selected by a call to
+ * qmessage_set_format.
+ *
+ * If @flags contains QMESSAGE_CONTEXT_SKIP_MONITOR
+ * an empty string will be returned if running in
+ * the context of a HMP command
+ *
+ * Returns: a formatted message prefix, or empty string;
+ * to be freed by the caller.
+ */
+char *qmessage_context(int flags);
+
+#endif /* QEMU_MESSAGE_H */
diff --git a/util/meson.build b/util/meson.build
index 35029380a3..f5365e3b4f 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -40,6 +40,7 @@ util_ss.add(files('host-utils.c'))
 util_ss.add(files('bitmap.c', 'bitops.c'))
 util_ss.add(files('fifo8.c'))
 util_ss.add(files('cacheflush.c'))
+util_ss.add(files('message.c'))
 util_ss.add(files('error.c', 'error-report.c'))
 util_ss.add(files('qemu-print.c'))
 util_ss.add(files('id.c'))
diff --git a/util/message.c b/util/message.c
new file mode 100644
index 0000000000..4c7eeb75e2
--- /dev/null
+++ b/util/message.c
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include "qemu/osdep.h"
+
+#include "qemu/message.h"
+#include "monitor/monitor.h"
+
+static int message_format;
+
+void qmessage_set_format(int flags)
+{
+    message_format = flags;
+}
+
+char *qmessage_context(int flags)
+{
+    g_autofree char *timestr = NULL;
+
+    if ((flags & QMESSAGE_CONTEXT_SKIP_MONITOR) &&
+        monitor_cur()) {
+        return g_strdup("");
+    }
+
+    if (message_format & QMESSAGE_FORMAT_TIMESTAMP) {
+        g_autoptr(GDateTime) dt = g_date_time_new_now_utc();
+        timestr = g_date_time_format_iso8601(dt);
+    }
+
+    return g_strdup_printf("%s%s",
+                           timestr ? timestr : "",
+                           timestr ? " " : "");
+}
-- 
2.50.1


Re: [PATCH 07/12] util: introduce common helper for error-report & log code
Posted by Richard Henderson 2 weeks, 5 days ago
On 8/20/25 06:27, Daniel P. Berrangé wrote:
> +char *qmessage_context(int flags)
> +{
> +    g_autofree char *timestr = NULL;
> +
> +    if ((flags & QMESSAGE_CONTEXT_SKIP_MONITOR) &&
> +        monitor_cur()) {
> +        return g_strdup("");

In the event we want no context, we really should avoid memory allocation too.


> +    }
> +
> +    if (message_format & QMESSAGE_FORMAT_TIMESTAMP) {
> +        g_autoptr(GDateTime) dt = g_date_time_new_now_utc();
> +        timestr = g_date_time_format_iso8601(dt);
> +    }
> +
> +    return g_strdup_printf("%s%s",
> +                           timestr ? timestr : "",
> +                           timestr ? " " : "");

g_strdup_printf isn't the nicest string concatenation tool.
And again, there's a no-context path.


r~

Re: [PATCH 07/12] util: introduce common helper for error-report & log code
Posted by Daniel P. Berrangé 2 weeks, 4 days ago
On Wed, Aug 20, 2025 at 07:57:47AM +1000, Richard Henderson wrote:
> On 8/20/25 06:27, Daniel P. Berrangé wrote:
> > +char *qmessage_context(int flags)
> > +{
> > +    g_autofree char *timestr = NULL;
> > +
> > +    if ((flags & QMESSAGE_CONTEXT_SKIP_MONITOR) &&
> > +        monitor_cur()) {
> > +        return g_strdup("");
> 
> In the event we want no context, we really should avoid memory allocation too.

Yes, won't not be too hard to check for NULL in the caller.

> > +    }
> > +
> > +    if (message_format & QMESSAGE_FORMAT_TIMESTAMP) {
> > +        g_autoptr(GDateTime) dt = g_date_time_new_now_utc();
> > +        timestr = g_date_time_format_iso8601(dt);
> > +    }
> > +
> > +    return g_strdup_printf("%s%s",
> > +                           timestr ? timestr : "",
> > +                           timestr ? " " : "");
> 
> g_strdup_printf isn't the nicest string concatenation tool.

A GString would make it easier. Though typically that would involve
an allocation of GSTring struct, I guess we could be sneaky and stack
allocate the struct.

> And again, there's a no-context path.



With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|