| Message ID | 20231005045041.52649-2-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: Kill 'qapi/qmp/qerror.h' for good | expand |
On 10/5/23 06:50, Philippe Mathieu-Daudé wrote: > 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> Reviewed-by: Cédric Le Goater <clg@kaod.org> Thanks, C. > --- > 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))); > 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", > 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; > } >
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.
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))); 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", 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; }
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(-)