|
From: | Daniel Henrique Barboza |
Subject: | Re: [PATCH v2 2/2] spapr: use DEVICE_UNPLUG_ERROR to report unplug errors |
Date: | Wed, 16 Jun 2021 13:58:04 -0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 |
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?
[Prev in Thread] | Current Thread | [Next in Thread] |