Re: [PATCH v2 01/15] qemu: Handle quirks of 'device' field of BLOCK_IO_ERROR event in monitor code

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(a)redhat.com&gt; > --- > 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(a)redhat.com&gt; > > 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.