On Mon, 12 Jul 2021 16:43:33 -0300
Daniel Henrique Barboza <danielhb...@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 <arm...@redhat.com>
> Signed-off-by: Daniel Henrique Barboza <danielhb...@gmail.com>
> ---
Appart from the nit in the changelog,
Reviewed-by: Greg Kurz <gr...@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;
> }
