On Tue, Jan 28, 2025 at 05:28:05PM +0100, Peter Krempa wrote:
> BLOCK_IO_ERROR's 'device' field is an empty string in case when it isn't
> applicable as it was originally mandatory in the qemu API docs.
>
> Move the logic that convert's empty string back to NULL from
> 'qemuProcessHandleIOError()' to 'qemuMonitorJSONHandleIOError()'
>
> This also fixes a hypothetical NULL-dereference if qemu would indeed
> report an IO error without the 'device' field present.
>
> Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
> ---
> src/qemu/qemu_monitor_json.c | 9 ++++++++-
> src/qemu/qemu_process.c | 3 ---
> 2 files changed, 8 insertions(+), 4 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
>
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 9f417d27c6..5ca76d2d80 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -708,8 +708,15 @@ qemuMonitorJSONHandleIOError(qemuMonitor *mon, virJSONValue *data)
> action = "ignore";
> }
>
> - if ((device = virJSONValueObjectGetString(data, "device")) == NULL)
> + if ((device = virJSONValueObjectGetString(data, "device")) == NULL) {
> VIR_WARN("missing device in disk io error event");
> + } else {
> + /* 'device' was documented as mandatory in the qemu event, but later became
> + * optional, in which case an empty string is sent by qemu. Convert it back
> + * to NULL */
> + if (*device == '\0')
> + device = NULL;
Oh, I'm surprised that QEMU doesn't send a JSON "null" in this case
already.
> + }
>
> nodename = virJSONValueObjectGetString(data, "node-name");
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 34a755a49a..edcd8ac3a8 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -840,9 +840,6 @@ qemuProcessHandleIOError(qemuMonitor *mon G_GNUC_UNUSED,
> virObjectLock(vm);
> priv = QEMU_DOMAIN_PRIVATE(vm);
>
> - if (*diskAlias == '\0')
> - diskAlias = NULL;
> -
> if (diskAlias)
> disk = qemuProcessFindDomainDiskByAliasOrQOM(vm, diskAlias, NULL);
> else if (nodename)
> --
> 2.48.1
>
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 :|
