| Message ID | 20210707003314.37110-4-danielhb413@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DEVICE_UNPLUG_ERROR QAPI event | expand |
On Tue, 6 Jul 2021 21:33:14 -0300 Daniel Henrique Barboza <danielhb413@gmail.com> wrote: > MEM_UNPLUG_ERROR is deprecated since the introduction of > DEVICE_UNPLUG_ERROR. Keep emitting both while the deprecation of > MEM_UNPLUG_ERROR is pending. > > CC: Michael S. Tsirkin <mst@redhat.com> > CC: Igor Mammedov <imammedo@redhat.com> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > --- Reviewed-by: Greg Kurz <groug@kaod.org> > hw/acpi/memory_hotplug.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c > index af37889423..fb9f4d2de7 100644 > --- a/hw/acpi/memory_hotplug.c > +++ b/hw/acpi/memory_hotplug.c > @@ -8,6 +8,7 @@ > #include "qapi/error.h" > #include "qapi/qapi-events-acpi.h" > #include "qapi/qapi-events-machine.h" > +#include "qapi/qapi-events-qdev.h" > > #define MEMORY_SLOTS_NUMBER "MDNR" > #define MEMORY_HOTPLUG_IO_REGION "HPMR" > @@ -177,9 +178,17 @@ 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)); > + > + /* > + * Send both MEM_UNPLUG_ERROR and DEVICE_UNPLUG_ERROR > + * while the deprecation of MEM_UNPLUG_ERROR is > + * pending. > + */ > + qapi_event_send_mem_unplug_error(dev->id, error_pretty); > + qapi_event_send_device_unplug_error(dev->id, error_pretty); > error_free(local_err); > break; > }
Daniel Henrique Barboza <danielhb413@gmail.com> writes: > MEM_UNPLUG_ERROR is deprecated since the introduction of > DEVICE_UNPLUG_ERROR. Keep emitting both while the deprecation of > MEM_UNPLUG_ERROR is pending. > > CC: Michael S. Tsirkin <mst@redhat.com> > CC: Igor Mammedov <imammedo@redhat.com> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > --- > hw/acpi/memory_hotplug.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c > index af37889423..fb9f4d2de7 100644 > --- a/hw/acpi/memory_hotplug.c > +++ b/hw/acpi/memory_hotplug.c > @@ -8,6 +8,7 @@ > #include "qapi/error.h" > #include "qapi/qapi-events-acpi.h" > #include "qapi/qapi-events-machine.h" > +#include "qapi/qapi-events-qdev.h" > > #define MEMORY_SLOTS_NUMBER "MDNR" > #define MEMORY_HOTPLUG_IO_REGION "HPMR" > @@ -177,9 +178,17 @@ 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)); > + > + /* > + * Send both MEM_UNPLUG_ERROR and DEVICE_UNPLUG_ERROR > + * while the deprecation of MEM_UNPLUG_ERROR is > + * pending. > + */ > + qapi_event_send_mem_unplug_error(dev->id, error_pretty); > + qapi_event_send_device_unplug_error(dev->id, error_pretty); > error_free(local_err); > break; > } Same question as for PATCH 2: can dev->id be null?
On Thu, 08 Jul 2021 15:08:57 +0200 Markus Armbruster <armbru@redhat.com> wrote: > Daniel Henrique Barboza <danielhb413@gmail.com> writes: > > > MEM_UNPLUG_ERROR is deprecated since the introduction of > > DEVICE_UNPLUG_ERROR. Keep emitting both while the deprecation of > > MEM_UNPLUG_ERROR is pending. > > > > CC: Michael S. Tsirkin <mst@redhat.com> > > CC: Igor Mammedov <imammedo@redhat.com> > > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > > --- > > hw/acpi/memory_hotplug.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c > > index af37889423..fb9f4d2de7 100644 > > --- a/hw/acpi/memory_hotplug.c > > +++ b/hw/acpi/memory_hotplug.c > > @@ -8,6 +8,7 @@ > > #include "qapi/error.h" > > #include "qapi/qapi-events-acpi.h" > > #include "qapi/qapi-events-machine.h" > > +#include "qapi/qapi-events-qdev.h" > > > > #define MEMORY_SLOTS_NUMBER "MDNR" > > #define MEMORY_HOTPLUG_IO_REGION "HPMR" > > @@ -177,9 +178,17 @@ 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)); > > + > > + /* > > + * Send both MEM_UNPLUG_ERROR and DEVICE_UNPLUG_ERROR > > + * while the deprecation of MEM_UNPLUG_ERROR is > > + * pending. > > + */ > > + qapi_event_send_mem_unplug_error(dev->id, error_pretty); > > + qapi_event_send_device_unplug_error(dev->id, error_pretty); > > error_free(local_err); > > break; > > } > > Same question as for PATCH 2: can dev->id be null? only theoretically (if memory device were created directly without using device_add), which as far as I know is not the case as all memory devices are created using -device/device_add so far. ( for device_add case see qdev_device_add->qdev_set_id where 'id' is set to user provided or to generated "device[%d]" value)
Igor Mammedov <imammedo@redhat.com> writes: > On Thu, 08 Jul 2021 15:08:57 +0200 > Markus Armbruster <armbru@redhat.com> wrote: > >> Daniel Henrique Barboza <danielhb413@gmail.com> writes: >> >> > MEM_UNPLUG_ERROR is deprecated since the introduction of >> > DEVICE_UNPLUG_ERROR. Keep emitting both while the deprecation of >> > MEM_UNPLUG_ERROR is pending. >> > >> > CC: Michael S. Tsirkin <mst@redhat.com> >> > CC: Igor Mammedov <imammedo@redhat.com> >> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> >> > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> >> > --- >> > hw/acpi/memory_hotplug.c | 13 +++++++++++-- >> > 1 file changed, 11 insertions(+), 2 deletions(-) >> > >> > diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c >> > index af37889423..fb9f4d2de7 100644 >> > --- a/hw/acpi/memory_hotplug.c >> > +++ b/hw/acpi/memory_hotplug.c >> > @@ -8,6 +8,7 @@ >> > #include "qapi/error.h" >> > #include "qapi/qapi-events-acpi.h" >> > #include "qapi/qapi-events-machine.h" >> > +#include "qapi/qapi-events-qdev.h" >> > >> > #define MEMORY_SLOTS_NUMBER "MDNR" >> > #define MEMORY_HOTPLUG_IO_REGION "HPMR" >> > @@ -177,9 +178,17 @@ 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)); >> > + >> > + /* >> > + * Send both MEM_UNPLUG_ERROR and DEVICE_UNPLUG_ERROR >> > + * while the deprecation of MEM_UNPLUG_ERROR is >> > + * pending. >> > + */ >> > + qapi_event_send_mem_unplug_error(dev->id, error_pretty); >> > + qapi_event_send_device_unplug_error(dev->id, error_pretty); >> > error_free(local_err); >> > break; >> > } >> >> Same question as for PATCH 2: can dev->id be null? > only theoretically (if memory device were created directly without > using device_add), which as far as I know is not the case as all > memory devices are created using -device/device_add so far. > > ( for device_add case see qdev_device_add->qdev_set_id where > 'id' is set to user provided or to generated "device[%d]" value) Something is set to a generated value, but it's not dev->id :) void qdev_set_id(DeviceState *dev, const char *id) @id is the value of id=... It may be null. dev->id still is null here. { if (id) { dev->id = id; } dev->id is now the value of id=... It may be null. if (dev->id) { object_property_add_child(qdev_get_peripheral(), dev->id, OBJECT(dev)); If the user specified id=..., add @dev as child of /peripheral. The child's name is the (non-null) value of id=... } else { static int anon_count; gchar *name = g_strdup_printf("device[%d]", anon_count++); object_property_add_child(qdev_get_peripheral_anon(), name, OBJECT(dev)); g_free(name); Else, add @dev as child of /peripheral-anon. The child's name is made up. } } dev->id is still the value of id=..., i.e. it may be null. Sure dereferencing dev->id in acpi_memory_hotplug_write() is safe?
On Fri, 09 Jul 2021 13:25:43 +0200 Markus Armbruster <armbru@redhat.com> wrote: > Igor Mammedov <imammedo@redhat.com> writes: > > > On Thu, 08 Jul 2021 15:08:57 +0200 > > Markus Armbruster <armbru@redhat.com> wrote: > > > >> Daniel Henrique Barboza <danielhb413@gmail.com> writes: > >> > >> > MEM_UNPLUG_ERROR is deprecated since the introduction of > >> > DEVICE_UNPLUG_ERROR. Keep emitting both while the deprecation of > >> > MEM_UNPLUG_ERROR is pending. > >> > > >> > CC: Michael S. Tsirkin <mst@redhat.com> > >> > CC: Igor Mammedov <imammedo@redhat.com> > >> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > >> > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > >> > --- > >> > hw/acpi/memory_hotplug.c | 13 +++++++++++-- > >> > 1 file changed, 11 insertions(+), 2 deletions(-) > >> > > >> > diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c > >> > index af37889423..fb9f4d2de7 100644 > >> > --- a/hw/acpi/memory_hotplug.c > >> > +++ b/hw/acpi/memory_hotplug.c > >> > @@ -8,6 +8,7 @@ > >> > #include "qapi/error.h" > >> > #include "qapi/qapi-events-acpi.h" > >> > #include "qapi/qapi-events-machine.h" > >> > +#include "qapi/qapi-events-qdev.h" > >> > > >> > #define MEMORY_SLOTS_NUMBER "MDNR" > >> > #define MEMORY_HOTPLUG_IO_REGION "HPMR" > >> > @@ -177,9 +178,17 @@ 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)); > >> > + > >> > + /* > >> > + * Send both MEM_UNPLUG_ERROR and DEVICE_UNPLUG_ERROR > >> > + * while the deprecation of MEM_UNPLUG_ERROR is > >> > + * pending. > >> > + */ > >> > + qapi_event_send_mem_unplug_error(dev->id, error_pretty); > >> > + qapi_event_send_device_unplug_error(dev->id, error_pretty); > >> > error_free(local_err); > >> > break; > >> > } > >> > >> Same question as for PATCH 2: can dev->id be null? > > only theoretically (if memory device were created directly without > > using device_add), which as far as I know is not the case as all > > memory devices are created using -device/device_add so far. > > > > ( for device_add case see qdev_device_add->qdev_set_id where > > 'id' is set to user provided or to generated "device[%d]" value) > > Something is set to a generated value, but it's not dev->id :) > > void qdev_set_id(DeviceState *dev, const char *id) > > @id is the value of id=... It may be null. > > dev->id still is null here. > > { > if (id) { > dev->id = id; > } > > dev->id is now the value of id=... It may be null. > > if (dev->id) { > object_property_add_child(qdev_get_peripheral(), dev->id, > OBJECT(dev)); > > If the user specified id=..., add @dev as child of /peripheral. The > child's name is the (non-null) value of id=... > > } else { > static int anon_count; > gchar *name = g_strdup_printf("device[%d]", anon_count++); > object_property_add_child(qdev_get_peripheral_anon(), name, > OBJECT(dev)); > g_free(name); > > Else, add @dev as child of /peripheral-anon. The child's name is made > up. > > > } > } > > dev->id is still the value of id=..., i.e. it may be null. yep, I was wrong and confused it child name in QOM tree. > Sure dereferencing dev->id in acpi_memory_hotplug_write() is safe? it aren't safe since guest may trigger this error when memory-device is created without id.
Igor Mammedov <imammedo@redhat.com> writes: > On Fri, 09 Jul 2021 13:25:43 +0200 > Markus Armbruster <armbru@redhat.com> wrote: > >> Igor Mammedov <imammedo@redhat.com> writes: >> >> > On Thu, 08 Jul 2021 15:08:57 +0200 >> > Markus Armbruster <armbru@redhat.com> wrote: >> > >> >> Daniel Henrique Barboza <danielhb413@gmail.com> writes: >> >> >> >> > MEM_UNPLUG_ERROR is deprecated since the introduction of >> >> > DEVICE_UNPLUG_ERROR. Keep emitting both while the deprecation of >> >> > MEM_UNPLUG_ERROR is pending. >> >> > >> >> > CC: Michael S. Tsirkin <mst@redhat.com> >> >> > CC: Igor Mammedov <imammedo@redhat.com> >> >> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> >> >> > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> >> >> > --- >> >> > hw/acpi/memory_hotplug.c | 13 +++++++++++-- >> >> > 1 file changed, 11 insertions(+), 2 deletions(-) >> >> > >> >> > diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c >> >> > index af37889423..fb9f4d2de7 100644 >> >> > --- a/hw/acpi/memory_hotplug.c >> >> > +++ b/hw/acpi/memory_hotplug.c >> >> > @@ -8,6 +8,7 @@ >> >> > #include "qapi/error.h" >> >> > #include "qapi/qapi-events-acpi.h" >> >> > #include "qapi/qapi-events-machine.h" >> >> > +#include "qapi/qapi-events-qdev.h" >> >> > >> >> > #define MEMORY_SLOTS_NUMBER "MDNR" >> >> > #define MEMORY_HOTPLUG_IO_REGION "HPMR" >> >> > @@ -177,9 +178,17 @@ 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)); >> >> > + >> >> > + /* >> >> > + * Send both MEM_UNPLUG_ERROR and DEVICE_UNPLUG_ERROR >> >> > + * while the deprecation of MEM_UNPLUG_ERROR is >> >> > + * pending. >> >> > + */ >> >> > + qapi_event_send_mem_unplug_error(dev->id, error_pretty); >> >> > + qapi_event_send_device_unplug_error(dev->id, error_pretty); >> >> > error_free(local_err); >> >> > break; >> >> > } >> >> >> >> Same question as for PATCH 2: can dev->id be null? >> > only theoretically (if memory device were created directly without >> > using device_add), which as far as I know is not the case as all >> > memory devices are created using -device/device_add so far. >> > >> > ( for device_add case see qdev_device_add->qdev_set_id where >> > 'id' is set to user provided or to generated "device[%d]" value) >> >> Something is set to a generated value, but it's not dev->id :) >> >> void qdev_set_id(DeviceState *dev, const char *id) >> >> @id is the value of id=... It may be null. >> >> dev->id still is null here. >> >> { >> if (id) { >> dev->id = id; >> } >> >> dev->id is now the value of id=... It may be null. >> >> if (dev->id) { >> object_property_add_child(qdev_get_peripheral(), dev->id, >> OBJECT(dev)); >> >> If the user specified id=..., add @dev as child of /peripheral. The >> child's name is the (non-null) value of id=... >> >> } else { >> static int anon_count; >> gchar *name = g_strdup_printf("device[%d]", anon_count++); >> object_property_add_child(qdev_get_peripheral_anon(), name, >> OBJECT(dev)); >> g_free(name); >> >> Else, add @dev as child of /peripheral-anon. The child's name is made >> up. >> >> >> } >> } >> >> dev->id is still the value of id=..., i.e. it may be null. > yep, I was wrong and confused it child name in QOM tree. > >> Sure dereferencing dev->id in acpi_memory_hotplug_write() is safe? > > it aren't safe since guest may trigger this error when > memory-device is created without id. Thanks! Daniel, the issue predates your series, but your series adds instances. We need a patch fixing the existing instances before your series, and fix up your series. Can you take care of that?
On 7/10/21 3:57 AM, Markus Armbruster wrote: > Igor Mammedov <imammedo@redhat.com> writes: > >> On Fri, 09 Jul 2021 13:25:43 +0200 >> Markus Armbruster <armbru@redhat.com> wrote: >> >>> Igor Mammedov <imammedo@redhat.com> writes: >>> >>>> On Thu, 08 Jul 2021 15:08:57 +0200 >>>> Markus Armbruster <armbru@redhat.com> wrote: >>>> >>>>> Daniel Henrique Barboza <danielhb413@gmail.com> writes: >>>>> >>>>>> MEM_UNPLUG_ERROR is deprecated since the introduction of >>>>>> DEVICE_UNPLUG_ERROR. Keep emitting both while the deprecation of >>>>>> MEM_UNPLUG_ERROR is pending. >>>>>> >>>>>> CC: Michael S. Tsirkin <mst@redhat.com> >>>>>> CC: Igor Mammedov <imammedo@redhat.com> >>>>>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> >>>>>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> >>>>>> --- >>>>>> hw/acpi/memory_hotplug.c | 13 +++++++++++-- >>>>>> 1 file changed, 11 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c >>>>>> index af37889423..fb9f4d2de7 100644 >>>>>> --- a/hw/acpi/memory_hotplug.c >>>>>> +++ b/hw/acpi/memory_hotplug.c >>>>>> @@ -8,6 +8,7 @@ >>>>>> #include "qapi/error.h" >>>>>> #include "qapi/qapi-events-acpi.h" >>>>>> #include "qapi/qapi-events-machine.h" >>>>>> +#include "qapi/qapi-events-qdev.h" >>>>>> >>>>>> #define MEMORY_SLOTS_NUMBER "MDNR" >>>>>> #define MEMORY_HOTPLUG_IO_REGION "HPMR" >>>>>> @@ -177,9 +178,17 @@ 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)); >>>>>> + >>>>>> + /* >>>>>> + * Send both MEM_UNPLUG_ERROR and DEVICE_UNPLUG_ERROR >>>>>> + * while the deprecation of MEM_UNPLUG_ERROR is >>>>>> + * pending. >>>>>> + */ >>>>>> + qapi_event_send_mem_unplug_error(dev->id, error_pretty); >>>>>> + qapi_event_send_device_unplug_error(dev->id, error_pretty); >>>>>> error_free(local_err); >>>>>> break; >>>>>> } >>>>> >>>>> Same question as for PATCH 2: can dev->id be null? >>>> only theoretically (if memory device were created directly without >>>> using device_add), which as far as I know is not the case as all >>>> memory devices are created using -device/device_add so far. >>>> >>>> ( for device_add case see qdev_device_add->qdev_set_id where >>>> 'id' is set to user provided or to generated "device[%d]" value) >>> >>> Something is set to a generated value, but it's not dev->id :) >>> >>> void qdev_set_id(DeviceState *dev, const char *id) >>> >>> @id is the value of id=... It may be null. >>> >>> dev->id still is null here. >>> >>> { >>> if (id) { >>> dev->id = id; >>> } >>> >>> dev->id is now the value of id=... It may be null. >>> >>> if (dev->id) { >>> object_property_add_child(qdev_get_peripheral(), dev->id, >>> OBJECT(dev)); >>> >>> If the user specified id=..., add @dev as child of /peripheral. The >>> child's name is the (non-null) value of id=... >>> >>> } else { >>> static int anon_count; >>> gchar *name = g_strdup_printf("device[%d]", anon_count++); >>> object_property_add_child(qdev_get_peripheral_anon(), name, >>> OBJECT(dev)); >>> g_free(name); >>> >>> Else, add @dev as child of /peripheral-anon. The child's name is made >>> up. >>> >>> >>> } >>> } >>> >>> dev->id is still the value of id=..., i.e. it may be null. >> yep, I was wrong and confused it child name in QOM tree. >> >>> Sure dereferencing dev->id in acpi_memory_hotplug_write() is safe? >> >> it aren't safe since guest may trigger this error when >> memory-device is created without id. > > Thanks! > > Daniel, the issue predates your series, but your series adds instances. > We need a patch fixing the existing instances before your series, and > fix up your series. Can you take care of that? Sure. I'll add a patch to handle the dev->id == NULL case before calling qapi_event_send_mem_unplug_error() in acpi_memory_hotplug_write(). Daniel >
diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c index af37889423..fb9f4d2de7 100644 --- a/hw/acpi/memory_hotplug.c +++ b/hw/acpi/memory_hotplug.c @@ -8,6 +8,7 @@ #include "qapi/error.h" #include "qapi/qapi-events-acpi.h" #include "qapi/qapi-events-machine.h" +#include "qapi/qapi-events-qdev.h" #define MEMORY_SLOTS_NUMBER "MDNR" #define MEMORY_HOTPLUG_IO_REGION "HPMR" @@ -177,9 +178,17 @@ 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)); + + /* + * Send both MEM_UNPLUG_ERROR and DEVICE_UNPLUG_ERROR + * while the deprecation of MEM_UNPLUG_ERROR is + * pending. + */ + qapi_event_send_mem_unplug_error(dev->id, error_pretty); + qapi_event_send_device_unplug_error(dev->id, error_pretty); error_free(local_err); break; }