On Thu, Jan 30, 2025 at 11:10:18 +0000, Daniel P. Berrangé wrote:
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@redhat.com> --- 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@redhat.com>
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.
The idea of sending empty string is to prevent API breakage as the 'device' field was documented to be mandatory and a string when BLOCK_IO_ERROR was initially added. Switching to JSON 'null' would be almost equivalent to just skipping the field (as virJSONValueObjectGetString would return NULL) at least for the above code in libvirt.