| Message ID | 20210825004835.472919-2-danielhb413@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | DEVICE_UNPLUG_GUEST_ERROR QAPI event | expand |
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;
On Tue, 24 Aug 2021 21:48:29 -0300 Daniel Henrique Barboza <danielhb413@gmail.com> 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: Igor Mammedov <imammedo@redhat.com> > --- > 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;
On Tue, 24 Aug 2021 21:48:29 -0300 Daniel Henrique Barboza <danielhb413@gmail.com> 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: Greg Kurz <groug@kaod.org> > 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;
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;
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> --- hw/acpi/memory_hotplug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)