On Fri, Jan 24, 2025 at 05:33:05PM +0100, Peter Krempa wrote:
> Pass the generic error message from qemu to the event so that users can
> possibly use the information. As documented the error is prefixed with
> "other: " to prevent collisions with already documented values in our
> API.
>
> Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
> ---
> src/qemu/qemu_monitor_json.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 9f417d27c6..6de34b03bb 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -695,7 +695,7 @@ qemuMonitorJSONHandleIOError(qemuMonitor *mon, virJSONValue *data)
> const char *device;
> const char *nodename;
> const char *action;
> - const char *reason = "";
> + g_autofree char *reason = NULL;
> bool nospc = false;
> int actionID;
>
> @@ -713,14 +713,27 @@ qemuMonitorJSONHandleIOError(qemuMonitor *mon, virJSONValue *data)
>
> nodename = virJSONValueObjectGetString(data, "node-name");
>
> - if (virJSONValueObjectGetBoolean(data, "nospace", &nospc) == 0 && nospc)
> - reason = "enospc";
> + if (virJSONValueObjectGetBoolean(data, "nospace", &nospc) == 0 && nospc) {
> + reason = g_strdup("enospc");
> + } else {
> + /* For any unknown error we prefix the qemu-originating error with 'other: '
> + * so that applications can use that to filter them from errors we
> + * define in our API */
> + const char *qemureason;
> +
> + if ((qemureason = virJSONValueObjectGetString(data, "reason"))) {
> + reason = g_strdup_printf("other: %s", qemureason);
> + }
> + }
>
> if ((actionID = qemuMonitorIOErrorActionTypeFromString(action)) < 0) {
> VIR_WARN("unknown disk io error action '%s'", action);
> actionID = VIR_DOMAIN_EVENT_IO_ERROR_NONE;
> }
>
> + if (!reason)
> + reason = g_strdup("");
> +
> qemuMonitorEmitIOError(mon, device, nodename, actionID, reason);
Per the last patch comments, we should expose the QMP "reason" as a separate
field from our current libvirt "reason" enum. This would prevent us exposing
it in the libvirt API for the IOError event, but we can still at least
record it in the log which apps/users will collect for bug reporting
purposes.
This isn't a huge loss, as we're already saying that even if we did expose
it in the API, apps shouldn't be looking at it, and we still have the option
to expose it in virDomainGetMessages later if desired.
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 :|
