| Message ID | 20211029230147.2465055-4-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/core: Remove uses of obsolete QERR_ definitions | expand |
Philippe Mathieu-Daudé <philmd@redhat.com> writes: > QERR_DEVICE_NO_HOTPLUG definition is obsolete since 2015 (commit > 4629ed1e989, "qerror: Finally unused, clean up"). Replace the two > uses and drop the definition. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > include/qapi/qmp/qerror.h | 3 --- > hw/core/qdev.c | 3 ++- > softmmu/qdev-monitor.c | 2 +- > 3 files changed, 3 insertions(+), 5 deletions(-) > > diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h > index 596fce0c54e..f49ae01cdb0 100644 > --- a/include/qapi/qmp/qerror.h > +++ b/include/qapi/qmp/qerror.h > @@ -26,9 +26,6 @@ > #define QERR_DEVICE_IN_USE \ > "Device '%s' is in use" > > -#define QERR_DEVICE_NO_HOTPLUG \ > - "Device '%s' does not support hotplugging" > - > #define QERR_FEATURE_DISABLED \ > "The feature '%s' is not enabled" > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index 7f06403752d..14375861c36 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -734,7 +734,8 @@ static void device_set_realized(Object *obj, bool value, Error **errp) > static int unattached_count; > > if (dev->hotplugged && !dc->hotpluggable) { > - error_setg(errp, QERR_DEVICE_NO_HOTPLUG, object_get_typename(obj)); > + error_setg(errp, "Device '%s' does not support hotplugging", > + object_get_typename(obj)); > return; > } > > diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c > index 4851de51a5c..35a885a6623 100644 > --- a/softmmu/qdev-monitor.c > +++ b/softmmu/qdev-monitor.c > @@ -896,7 +896,7 @@ void qdev_unplug(DeviceState *dev, Error **errp) > } > > if (!dc->hotpluggable) { > - error_setg(errp, QERR_DEVICE_NO_HOTPLUG, > + error_setg(errp, "Device '%s' does not support hotplugging", > object_get_typename(OBJECT(dev))); > return; > } When the same error is detected in multiple places, I like to ask myself whether the code calls for a refactoring. But I can't see a useful one here. Reviewed-by: Markus Armbruster <armbru@redhat.com>
On 11/19/21 09:20, Markus Armbruster wrote: > Philippe Mathieu-Daudé <philmd@redhat.com> writes: > >> QERR_DEVICE_NO_HOTPLUG definition is obsolete since 2015 (commit >> 4629ed1e989, "qerror: Finally unused, clean up"). Replace the two >> uses and drop the definition. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> include/qapi/qmp/qerror.h | 3 --- >> hw/core/qdev.c | 3 ++- >> softmmu/qdev-monitor.c | 2 +- >> 3 files changed, 3 insertions(+), 5 deletions(-) >> >> diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h >> index 596fce0c54e..f49ae01cdb0 100644 >> --- a/include/qapi/qmp/qerror.h >> +++ b/include/qapi/qmp/qerror.h >> @@ -26,9 +26,6 @@ >> #define QERR_DEVICE_IN_USE \ >> "Device '%s' is in use" >> >> -#define QERR_DEVICE_NO_HOTPLUG \ >> - "Device '%s' does not support hotplugging" >> - >> #define QERR_FEATURE_DISABLED \ >> "The feature '%s' is not enabled" >> >> diff --git a/hw/core/qdev.c b/hw/core/qdev.c >> index 7f06403752d..14375861c36 100644 >> --- a/hw/core/qdev.c >> +++ b/hw/core/qdev.c >> @@ -734,7 +734,8 @@ static void device_set_realized(Object *obj, bool value, Error **errp) >> static int unattached_count; >> >> if (dev->hotplugged && !dc->hotpluggable) { >> - error_setg(errp, QERR_DEVICE_NO_HOTPLUG, object_get_typename(obj)); >> + error_setg(errp, "Device '%s' does not support hotplugging", >> + object_get_typename(obj)); >> return; >> } >> >> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c >> index 4851de51a5c..35a885a6623 100644 >> --- a/softmmu/qdev-monitor.c >> +++ b/softmmu/qdev-monitor.c >> @@ -896,7 +896,7 @@ void qdev_unplug(DeviceState *dev, Error **errp) >> } >> >> if (!dc->hotpluggable) { >> - error_setg(errp, QERR_DEVICE_NO_HOTPLUG, >> + error_setg(errp, "Device '%s' does not support hotplugging", >> object_get_typename(OBJECT(dev))); >> return; >> } > > When the same error is detected in multiple places, I like to ask myself > whether the code calls for a refactoring. But I can't see a useful one > here. > Since the realize check will catch more case (devices created recursively by a command or simply by C code), we could remove the softmmu/qdev-monitor.c check and rely on realize catching that just after. In theory failing during realize should have the same result as "not trying to create the device in the first place". -- Damien
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index 596fce0c54e..f49ae01cdb0 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -26,9 +26,6 @@ #define QERR_DEVICE_IN_USE \ "Device '%s' is in use" -#define QERR_DEVICE_NO_HOTPLUG \ - "Device '%s' does not support hotplugging" - #define QERR_FEATURE_DISABLED \ "The feature '%s' is not enabled" diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 7f06403752d..14375861c36 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -734,7 +734,8 @@ static void device_set_realized(Object *obj, bool value, Error **errp) static int unattached_count; if (dev->hotplugged && !dc->hotpluggable) { - error_setg(errp, QERR_DEVICE_NO_HOTPLUG, object_get_typename(obj)); + error_setg(errp, "Device '%s' does not support hotplugging", + object_get_typename(obj)); return; } diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c index 4851de51a5c..35a885a6623 100644 --- a/softmmu/qdev-monitor.c +++ b/softmmu/qdev-monitor.c @@ -896,7 +896,7 @@ void qdev_unplug(DeviceState *dev, Error **errp) } if (!dc->hotpluggable) { - error_setg(errp, QERR_DEVICE_NO_HOTPLUG, + error_setg(errp, "Device '%s' does not support hotplugging", object_get_typename(OBJECT(dev))); return; }
QERR_DEVICE_NO_HOTPLUG definition is obsolete since 2015 (commit 4629ed1e989, "qerror: Finally unused, clean up"). Replace the two uses and drop the definition. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- include/qapi/qmp/qerror.h | 3 --- hw/core/qdev.c | 3 ++- softmmu/qdev-monitor.c | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-)