Re: [PATCH v7 1/7] memory_hotplug.c: handle dev->id = NULL in acpi_memory_hotplug_write()

[David Gibson]

On Tue, Aug 24, 2021 at 09:48:29PM -0300, Daniel Henrique Barboza wrote:
> qapi_event_send_mem_unplug_error() deals with @device being NULL by
> replacing it with an empty string ("") when emitting the event. Aside
> from the fact that this behavior (qapi visitor mapping NULL pointer to
> "") can be patched/changed someday, there's also the lack of utility
> that the event brings to listeners, e.g. "a memory unplug error happened
> somewhere".
> 
> In theory we should just avoit emitting this event at all if dev->id is
> NULL, but this would be an incompatible change to existing guests.
> Instead, let's make the forementioned behavior explicit: if dev->id is
> NULL, pass an empty string to qapi_event_send_mem_unplug_error().
> 
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/acpi/memory_hotplug.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index af37889423..6a71de408b 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -178,7 +178,7 @@ static void acpi_memory_hotplug_write(void *opaque, 
> hwaddr addr, uint64_t data,
>              hotplug_handler_unplug(hotplug_ctrl, dev, &local_err);
>              if (local_err) {
>                  trace_mhp_acpi_pc_dimm_delete_failed(mem_st->selector);
> -                qapi_event_send_mem_unplug_error(dev->id,
> +                qapi_event_send_mem_unplug_error(dev->id ? : "",
>                                                   
> error_get_pretty(local_err));
>                  error_free(local_err);
>                  break;

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature