[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 01/22] qapi: Inline and remove QERR_BUS_NO_HOTPLUG definit
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v2 01/22] qapi: Inline and remove QERR_BUS_NO_HOTPLUG definition |
Date: |
Fri, 20 Oct 2023 07:49:29 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> Address the comment added in commit 4629ed1e98
> ("qerror: Finally unused, clean up"), from 2015:
>
> /*
> * These macros will go away, please don't use
> * in new code, and do not add new ones!
> */
>
> Mechanical transformation using sed, manually
> removing the definition in include/qapi/qmp/qerror.h.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> include/qapi/qmp/qerror.h | 3 ---
> hw/ppc/spapr_pci.c | 4 ++--
> softmmu/qdev-monitor.c | 8 +++++---
> 3 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
> index 8dd9fcb071..1a9c2d3502 100644
> --- a/include/qapi/qmp/qerror.h
> +++ b/include/qapi/qmp/qerror.h
> @@ -17,9 +17,6 @@
> * add new ones!
> */
>
> -#define QERR_BUS_NO_HOTPLUG \
> - "Bus '%s' does not support hotplugging"
> -
> #define QERR_DEVICE_HAS_NO_MEDIUM \
> "Device '%s' has no medium"
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 370c5a90f2..7f063f5852 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1550,7 +1550,7 @@ static void spapr_pci_pre_plug(HotplugHandler
> *plug_handler,
> * we need to let them know it's not enabled
> */
> if (plugged_dev->hotplugged) {
> - error_setg(errp, QERR_BUS_NO_HOTPLUG,
> + error_setg(errp, "Bus '%s' does not support hotplugging",
> object_get_typename(OBJECT(phb)));
@phb is a SpaprPhbState *, thereforce object_get_typename() returns
"spapr-pci-host-bridge", which is not a bus. It provides a bus: PCI bus
phb->parent_obj.bus.
Should be fixed on top, so this patch remains mechanical.
Well outside this patch's scope, but here goes anyway: I wonder why we
need this check. Why is the generic check in
qdev_device_add_from_qdict() not enough?
> return;
> }
> @@ -1671,7 +1671,7 @@ static void spapr_pci_unplug_request(HotplugHandler
> *plug_handler,
> SpaprDrc *drc = drc_from_dev(phb, pdev);
>
> if (!phb->dr_enabled) {
> - error_setg(errp, QERR_BUS_NO_HOTPLUG,
> + error_setg(errp, "Bus '%s' does not support hotplugging",
Likewise.
> object_get_typename(OBJECT(phb)));
> return;
> }
> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> index 74f4e41338..3a9740dcbd 100644
> --- a/softmmu/qdev-monitor.c
> +++ b/softmmu/qdev-monitor.c
> @@ -656,7 +656,8 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,
>
> if (qdev_should_hide_device(opts, from_json, errp)) {
> if (bus && !qbus_is_hotpluggable(bus)) {
> - error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name);
> + error_setg(errp, "Bus '%s' does not support hotplugging",
> + bus->name);
> }
> return NULL;
> } else if (*errp) {
> @@ -664,7 +665,7 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,
> }
>
> if (phase_check(PHASE_MACHINE_READY) && bus &&
> !qbus_is_hotpluggable(bus)) {
> - error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name);
> + error_setg(errp, "Bus '%s' does not support hotplugging", bus->name);
> return NULL;
> }
>
> @@ -904,7 +905,8 @@ void qdev_unplug(DeviceState *dev, Error **errp)
> }
>
> if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) {
> - error_setg(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
> + error_setg(errp, "Bus '%s' does not support hotplugging",
> + dev->parent_bus->name);
> return;
> }
Could factor out
if (bus && !qbus_is_hotpluggable(bus)) {
error_setg(errp, "Bus '%s' does not support hotplugging", bus->name);
return false;
}
return true;
Idea, not a demand.
- [PATCH v2 00/22] qapi: Kill 'qapi/qmp/qerror.h' for good, Philippe Mathieu-Daudé, 2023/10/05
- [PATCH v2 01/22] qapi: Inline and remove QERR_BUS_NO_HOTPLUG definition, Philippe Mathieu-Daudé, 2023/10/05
- [PATCH v2 03/22] qapi: Inline and remove QERR_DEVICE_IN_USE definition, Philippe Mathieu-Daudé, 2023/10/05
- [PATCH v2 02/22] qapi: Inline and remove QERR_DEVICE_HAS_NO_MEDIUM definition, Philippe Mathieu-Daudé, 2023/10/05
- [PATCH v2 04/22] qapi: Inline and remove QERR_DEVICE_NO_HOTPLUG definition, Philippe Mathieu-Daudé, 2023/10/05
- [PATCH v2 05/22] qapi: Inline QERR_INVALID_PARAMETER definition (constant parameter), Philippe Mathieu-Daudé, 2023/10/05
- [PATCH v2 07/22] qapi: Inline QERR_INVALID_PARAMETER_TYPE definition (constant param), Philippe Mathieu-Daudé, 2023/10/05
- [PATCH v2 06/22] qapi: Inline and remove QERR_INVALID_PARAMETER definition, Philippe Mathieu-Daudé, 2023/10/05