| Message ID | 20210712194339.813152-2-danielhb413@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DEVICE_UNPLUG_ERROR QAPI event | expand |
On Mon, 12 Jul 2021 16:43:33 -0300 Daniel Henrique Barboza <danielhb413@gmail.com> wrote: > qapi_event_send_device_unplug_error() deals with @device being NULL s/qapi_event_send_device_unplug_error/qapi_event_send_mem_unplug_error ? since I only see qapi_event_send_mem_unplug_error() in the diff. > by replacing it with an empty string ("") when emitting the event. Aside > from the fact that this is a side effect that can be patched someday, > there's also the lack of utility that the event brings to listeners, e.g. > "a memory unplug error happened somewhere". > > We're better of not emitting the event if dev->id is NULL. Next patches > will introduce a new device unplug error event that is better suited to > deal with dev->id NULL scenarios. MEM_UNPLUG_ERROR will continue to be > emitted to avoid breaking existing APIs, but it'll be deprecated and > removed in the future. > > Suggested-by: Markus Armbruster <armbru@redhat.com> > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > --- Appart from the nit in the changelog, Reviewed-by: Greg Kurz <groug@kaod.org> > hw/acpi/memory_hotplug.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c > index af37889423..e37acb0367 100644 > --- a/hw/acpi/memory_hotplug.c > +++ b/hw/acpi/memory_hotplug.c > @@ -177,9 +177,14 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data, > /* call pc-dimm unplug cb */ > hotplug_handler_unplug(hotplug_ctrl, dev, &local_err); > if (local_err) { > + const char *error_pretty = error_get_pretty(local_err); > + > trace_mhp_acpi_pc_dimm_delete_failed(mem_st->selector); > - qapi_event_send_mem_unplug_error(dev->id, > - error_get_pretty(local_err)); > + > + if (dev->id) { > + qapi_event_send_mem_unplug_error(dev->id, error_pretty); > + } > + > error_free(local_err); > break; > }
diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c index af37889423..e37acb0367 100644 --- a/hw/acpi/memory_hotplug.c +++ b/hw/acpi/memory_hotplug.c @@ -177,9 +177,14 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data, /* call pc-dimm unplug cb */ hotplug_handler_unplug(hotplug_ctrl, dev, &local_err); if (local_err) { + const char *error_pretty = error_get_pretty(local_err); + trace_mhp_acpi_pc_dimm_delete_failed(mem_st->selector); - qapi_event_send_mem_unplug_error(dev->id, - error_get_pretty(local_err)); + + if (dev->id) { + qapi_event_send_mem_unplug_error(dev->id, error_pretty); + } + error_free(local_err); break; }
qapi_event_send_device_unplug_error() deals with @device being NULL by replacing it with an empty string ("") when emitting the event. Aside from the fact that this is a side effect that can be patched someday, there's also the lack of utility that the event brings to listeners, e.g. "a memory unplug error happened somewhere". We're better of not emitting the event if dev->id is NULL. Next patches will introduce a new device unplug error event that is better suited to deal with dev->id NULL scenarios. MEM_UNPLUG_ERROR will continue to be emitted to avoid breaking existing APIs, but it'll be deprecated and removed in the future. Suggested-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- hw/acpi/memory_hotplug.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)