[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/2] spapr: use DEVICE_UNPLUG_ERROR to report unplug error
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v2 2/2] spapr: use DEVICE_UNPLUG_ERROR to report unplug errors |
Date: |
Fri, 11 Jun 2021 14:18:56 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) |
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?
[PATCH v2 2/2] spapr: use DEVICE_UNPLUG_ERROR to report unplug errors, Daniel Henrique Barboza, 2021/06/04
Re: [PATCH v2 0/2] DEVICE_UNPLUG_ERROR QAPI event, David Gibson, 2021/06/06