Linux Kernel 5.12 is now unisolating CPU DRCs in the device_removal
error path, signalling that the hotunplug process wasn't successful.
This allow us to send a DEVICE_UNPLUG_ERROR in drc_unisolate_logical()
to signal this error to the management layer.
We also have another error path in spapr_memory_unplug_rollback() for
configured LMB DRCs. Kernels older than 5.13 will not unisolate the LMBs
in the hotunplug error path, but it will reconfigure them. Let's send
the DEVICE_UNPLUG_ERROR event in that code path as well to cover the
case of older kernels.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
hw/ppc/spapr.c | 2 +-
hw/ppc/spapr_drc.c | 15 +++++++++------
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index c23bcc4490..29aa2f467d 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3639,7 +3639,7 @@ void spapr_memory_unplug_rollback(SpaprMachineState *spapr, DeviceState *dev)
*/
qapi_error = g_strdup_printf("Memory hotunplug rejected by the guest "
"for device %s", dev->id);
- qapi_event_send_mem_unplug_error(dev->id, qapi_error);
+ qapi_event_send_device_unplug_error(dev->id, qapi_error);
}
/* Callback to be called during DRC release. */
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index a2f2634601..0e1a8733bc 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -17,6 +17,8 @@
#include "hw/ppc/spapr_drc.h"
#include "qom/object.h"
#include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "qapi/qapi-events-machine.h"
#include "qapi/visitor.h"
#include "qemu/error-report.h"
#include "hw/ppc/spapr.h" /* for RTAS return codes */
@@ -160,6 +162,10 @@ static uint32_t drc_unisolate_logical(SpaprDrc *drc)
* means that the kernel is refusing the removal.
*/
if (drc->unplug_requested && drc->dev) {
+ const char qapi_error_fmt[] = "Device hotunplug rejected by the "
+ "guest for device %s";
+ g_autofree char *qapi_error = NULL;
+
if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_LMB) {
spapr = SPAPR_MACHINE(qdev_get_machine());
@@ -167,13 +173,10 @@ static uint32_t drc_unisolate_logical(SpaprDrc *drc)
}
drc->unplug_requested = false;
- error_report("Device hotunplug rejected by the guest "
- "for device %s", drc->dev->id);
+ error_report(qapi_error_fmt, drc->dev->id);
- /*
- * TODO: send a QAPI DEVICE_UNPLUG_ERROR event when
- * it is implemented.
- */
+ qapi_error = g_strdup_printf(qapi_error_fmt, drc->dev->id);
+ qapi_event_send_device_unplug_error(drc->dev->id, qapi_error);
}
return RTAS_OUT_SUCCESS; /* Nothing to do */
--
2.31.1
On Fri, Jun 04, 2021 at 05:03:53PM -0300, Daniel Henrique Barboza wrote:
> Linux Kernel 5.12 is now unisolating CPU DRCs in the device_removal
> error path, signalling that the hotunplug process wasn't successful.
> This allow us to send a DEVICE_UNPLUG_ERROR in drc_unisolate_logical()
> to signal this error to the management layer.
>
> We also have another error path in spapr_memory_unplug_rollback() for
> configured LMB DRCs. Kernels older than 5.13 will not unisolate the LMBs
> in the hotunplug error path, but it will reconfigure them. Let's send
> the DEVICE_UNPLUG_ERROR event in that code path as well to cover the
> case of older kernels.
>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/ppc/spapr.c | 2 +-
> hw/ppc/spapr_drc.c | 15 +++++++++------
> 2 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index c23bcc4490..29aa2f467d 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3639,7 +3639,7 @@ void spapr_memory_unplug_rollback(SpaprMachineState *spapr, DeviceState *dev)
> */
> qapi_error = g_strdup_printf("Memory hotunplug rejected by the guest "
> "for device %s", dev->id);
> - qapi_event_send_mem_unplug_error(dev->id, qapi_error);
> + qapi_event_send_device_unplug_error(dev->id, qapi_error);
> }
>
> /* Callback to be called during DRC release. */
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index a2f2634601..0e1a8733bc 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -17,6 +17,8 @@
> #include "hw/ppc/spapr_drc.h"
> #include "qom/object.h"
> #include "migration/vmstate.h"
> +#include "qapi/error.h"
> +#include "qapi/qapi-events-machine.h"
> #include "qapi/visitor.h"
> #include "qemu/error-report.h"
> #include "hw/ppc/spapr.h" /* for RTAS return codes */
> @@ -160,6 +162,10 @@ static uint32_t drc_unisolate_logical(SpaprDrc *drc)
> * means that the kernel is refusing the removal.
> */
> if (drc->unplug_requested && drc->dev) {
> + const char qapi_error_fmt[] = "Device hotunplug rejected by the "
> + "guest for device %s";
> + g_autofree char *qapi_error = NULL;
> +
> if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_LMB) {
> spapr = SPAPR_MACHINE(qdev_get_machine());
>
> @@ -167,13 +173,10 @@ static uint32_t drc_unisolate_logical(SpaprDrc *drc)
> }
>
> drc->unplug_requested = false;
> - error_report("Device hotunplug rejected by the guest "
> - "for device %s", drc->dev->id);
> + error_report(qapi_error_fmt, drc->dev->id);
>
> - /*
> - * TODO: send a QAPI DEVICE_UNPLUG_ERROR event when
> - * it is implemented.
> - */
> + qapi_error = g_strdup_printf(qapi_error_fmt, drc->dev->id);
> + qapi_event_send_device_unplug_error(drc->dev->id, qapi_error);
> }
>
> return RTAS_OUT_SUCCESS; /* Nothing to do */
--
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
Daniel Henrique Barboza <danielhb413@gmail.com> writes:
> Linux Kernel 5.12 is now unisolating CPU DRCs in the device_removal
> error path, signalling that the hotunplug process wasn't successful.
> This allow us to send a DEVICE_UNPLUG_ERROR in drc_unisolate_logical()
> to signal this error to the management layer.
>
> We also have another error path in spapr_memory_unplug_rollback() for
> configured LMB DRCs. Kernels older than 5.13 will not unisolate the LMBs
> in the hotunplug error path, but it will reconfigure them. Let's send
> the DEVICE_UNPLUG_ERROR event in that code path as well to cover the
> case of older kernels.
>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
> hw/ppc/spapr.c | 2 +-
> hw/ppc/spapr_drc.c | 15 +++++++++------
> 2 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index c23bcc4490..29aa2f467d 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3639,7 +3639,7 @@ void spapr_memory_unplug_rollback(SpaprMachineState *spapr, DeviceState *dev)
> */
> qapi_error = g_strdup_printf("Memory hotunplug rejected by the guest "
> "for device %s", dev->id);
> - qapi_event_send_mem_unplug_error(dev->id, qapi_error);
> + qapi_event_send_device_unplug_error(dev->id, qapi_error);
Incompatible change: we now emit DEVICE_UNPLUG_ERROR instead of
MEM_UNPLUG_ERROR. Intentional?
If yes, we need a release note.
To avoid the incompatible, we can emit both, and deprecate
MEM_UNPLUG_ERROR.
What about the MEM_UNPLUG_ERROR in acpi_memory_hotplug_write()?
> }
>
> /* Callback to be called during DRC release. */
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index a2f2634601..0e1a8733bc 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -17,6 +17,8 @@
> #include "hw/ppc/spapr_drc.h"
> #include "qom/object.h"
> #include "migration/vmstate.h"
> +#include "qapi/error.h"
> +#include "qapi/qapi-events-machine.h"
> #include "qapi/visitor.h"
> #include "qemu/error-report.h"
> #include "hw/ppc/spapr.h" /* for RTAS return codes */
> @@ -160,6 +162,10 @@ static uint32_t drc_unisolate_logical(SpaprDrc *drc)
> * means that the kernel is refusing the removal.
> */
> if (drc->unplug_requested && drc->dev) {
> + const char qapi_error_fmt[] = "Device hotunplug rejected by the "
> + "guest for device %s";
> + g_autofree char *qapi_error = NULL;
> +
> if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_LMB) {
> spapr = SPAPR_MACHINE(qdev_get_machine());
>
> @@ -167,13 +173,10 @@ static uint32_t drc_unisolate_logical(SpaprDrc *drc)
> }
>
> drc->unplug_requested = false;
> - error_report("Device hotunplug rejected by the guest "
> - "for device %s", drc->dev->id);
> + error_report(qapi_error_fmt, drc->dev->id);
>
> - /*
> - * TODO: send a QAPI DEVICE_UNPLUG_ERROR event when
> - * it is implemented.
> - */
> + qapi_error = g_strdup_printf(qapi_error_fmt, drc->dev->id);
> + qapi_event_send_device_unplug_error(drc->dev->id, qapi_error);
> }
>
> return RTAS_OUT_SUCCESS; /* Nothing to do */
Reporting both to stderr and QMP is odd. Can you describe a use case
where the report to stderr is useful?
On 6/11/21 9:18 AM, Markus Armbruster wrote:
> Daniel Henrique Barboza <danielhb413@gmail.com> writes:
>
>> Linux Kernel 5.12 is now unisolating CPU DRCs in the device_removal
>> error path, signalling that the hotunplug process wasn't successful.
>> This allow us to send a DEVICE_UNPLUG_ERROR in drc_unisolate_logical()
>> to signal this error to the management layer.
>>
>> We also have another error path in spapr_memory_unplug_rollback() for
>> configured LMB DRCs. Kernels older than 5.13 will not unisolate the LMBs
>> in the hotunplug error path, but it will reconfigure them. Let's send
>> the DEVICE_UNPLUG_ERROR event in that code path as well to cover the
>> case of older kernels.
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> ---
>> hw/ppc/spapr.c | 2 +-
>> hw/ppc/spapr_drc.c | 15 +++++++++------
>> 2 files changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index c23bcc4490..29aa2f467d 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -3639,7 +3639,7 @@ void spapr_memory_unplug_rollback(SpaprMachineState *spapr, DeviceState *dev)
>> */
>> qapi_error = g_strdup_printf("Memory hotunplug rejected by the guest "
>> "for device %s", dev->id);
>> - qapi_event_send_mem_unplug_error(dev->id, qapi_error);
>> + qapi_event_send_device_unplug_error(dev->id, qapi_error);
>
> Incompatible change: we now emit DEVICE_UNPLUG_ERROR instead of
> MEM_UNPLUG_ERROR. Intentional?
>
> If yes, we need a release note.
>
> To avoid the incompatible, we can emit both, and deprecate
> MEM_UNPLUG_ERROR.
>
> What about the MEM_UNPLUG_ERROR in acpi_memory_hotplug_write()?
I'll emit DEVICE_UNPLUG_ERROR together with all MEM_UNPLUG_ERROR instances.
Then we can deprecate MEM_UNPLUG_ERROR.
By the way, how do I mark MEM_UNPLUG_ERROR as deprecated? I see examples
of command line options being documented as deprecated in
docs/system/deprecated.rst and some deprecated QOM/QDEV properties are
marked as deprecated directly in their .json files, but I didn't find
any case where a whole event is deprecated. Would something like this be
adequate?
$ git diff
diff --git a/qapi/machine.json b/qapi/machine.json
index 58a9c86b36..ce3d873c64 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1261,6 +1261,10 @@
#
# @msg: Informative message
#
+#
+# @deprecated: Starting in 6.1 this event has been replaced by
+# DEVICE_UNPLUG_ERROR.
+#
# Since: 2.4
#
# Example:
Thanks,
Daniel
>
>> }
>>
>> /* Callback to be called during DRC release. */
>> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
>> index a2f2634601..0e1a8733bc 100644
>> --- a/hw/ppc/spapr_drc.c
>> +++ b/hw/ppc/spapr_drc.c
>> @@ -17,6 +17,8 @@
>> #include "hw/ppc/spapr_drc.h"
>> #include "qom/object.h"
>> #include "migration/vmstate.h"
>> +#include "qapi/error.h"
>> +#include "qapi/qapi-events-machine.h"
>> #include "qapi/visitor.h"
>> #include "qemu/error-report.h"
>> #include "hw/ppc/spapr.h" /* for RTAS return codes */
>> @@ -160,6 +162,10 @@ static uint32_t drc_unisolate_logical(SpaprDrc *drc)
>> * means that the kernel is refusing the removal.
>> */
>> if (drc->unplug_requested && drc->dev) {
>> + const char qapi_error_fmt[] = "Device hotunplug rejected by the "
>> + "guest for device %s";
>> + g_autofree char *qapi_error = NULL;
>> +
>> if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_LMB) {
>> spapr = SPAPR_MACHINE(qdev_get_machine());
>>
>> @@ -167,13 +173,10 @@ static uint32_t drc_unisolate_logical(SpaprDrc *drc)
>> }
>>
>> drc->unplug_requested = false;
>> - error_report("Device hotunplug rejected by the guest "
>> - "for device %s", drc->dev->id);
>> + error_report(qapi_error_fmt, drc->dev->id);
>>
>> - /*
>> - * TODO: send a QAPI DEVICE_UNPLUG_ERROR event when
>> - * it is implemented.
>> - */
>> + qapi_error = g_strdup_printf(qapi_error_fmt, drc->dev->id);
>> + qapi_event_send_device_unplug_error(drc->dev->id, qapi_error);
>> }
>>
>> return RTAS_OUT_SUCCESS; /* Nothing to do */
>
> Reporting both to stderr and QMP is odd. Can you describe a use case
> where the report to stderr is useful?
>
On Wed, Jun 16, 2021 at 01:58:04PM -0300, Daniel Henrique Barboza wrote: > > Incompatible change: we now emit DEVICE_UNPLUG_ERROR instead of > > MEM_UNPLUG_ERROR. Intentional? > > > > If yes, we need a release note. > > > > To avoid the incompatible, we can emit both, and deprecate > > MEM_UNPLUG_ERROR. > > > > What about the MEM_UNPLUG_ERROR in acpi_memory_hotplug_write()? > > I'll emit DEVICE_UNPLUG_ERROR together with all MEM_UNPLUG_ERROR instances. > Then we can deprecate MEM_UNPLUG_ERROR. > > By the way, how do I mark MEM_UNPLUG_ERROR as deprecated? I see examples > of command line options being documented as deprecated in > docs/system/deprecated.rst and some deprecated QOM/QDEV properties are > marked as deprecated directly in their .json files, but I didn't find > any case where a whole event is deprecated. Would something like this be > adequate? Almost. That documents the deprecation for readers, but you also need to mark it for viewing by machine code... > > > $ git diff > diff --git a/qapi/machine.json b/qapi/machine.json > index 58a9c86b36..ce3d873c64 100644 > --- a/qapi/machine.json > +++ b/qapi/machine.json > @@ -1261,6 +1261,10 @@ > # > # @msg: Informative message > # > +# > +# @deprecated: Starting in 6.1 this event has been replaced by > +# DEVICE_UNPLUG_ERROR. > +# > # Since: 2.4 > # > # Example: ...do that by adding 'features':['deprecated'] to the QAPI event definition. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
© 2016 - 2026 Red Hat, Inc.