On 07.03.24 12:46, Markus Armbruster wrote:
Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru> writes:
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
---
system/qdev-monitor.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 9febb743f1..cf7481e416 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -877,13 +877,20 @@ void qmp_device_add(QDict *qdict, QObject **ret_data,
Error **errp)
object_unref(OBJECT(dev));
}
-static DeviceState *find_device_state(const char *id, Error **errp)
+/*
+ * Note that creating new APIs using error classes other than GenericError is
+ * not recommended. Set use_generic_error=true for new interfaces.
+ */
+static DeviceState *find_device_state(const char *id, bool use_generic_error,
+ Error **errp)
{
Object *obj = object_resolve_path_at(qdev_get_peripheral(), id);
DeviceState *dev;
if (!obj) {
- error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+ error_set(errp,
+ (use_generic_error ?
+ ERROR_CLASS_GENERIC_ERROR : ERROR_CLASS_DEVICE_NOT_FOUND),
"Device '%s' not found", id);
return NULL;
}
@@ -947,7 +954,7 @@ void qdev_unplug(DeviceState *dev, Error **errp)
void qmp_device_del(const char *id, Error **errp)
{
- DeviceState *dev = find_device_state(id, errp);
+ DeviceState *dev = find_device_state(id, false, errp);
if (dev != NULL) {
if (dev->pending_deleted_event &&
(dev->pending_deleted_expires_ms == 0 ||
@@ -1067,7 +1074,7 @@ BlockBackend *blk_by_qdev_id(const char *id, Error **errp)
GLOBAL_STATE_CODE();
- dev = find_device_state(id, errp);
+ dev = find_device_state(id, false, errp);
if (dev == NULL) {
return NULL;
}
I appreciate the attempt to curb the spread of DeviceNotFound errors.
Two issues:
* Copy-pasting find_device_state() with a false argument is an easy
error to make.
* Most uses of find_device_state() are via blk_by_qdev_id() and
qmp_get_blk(). Any new uses of qemu_get_blk() will still produce
DeviceNotFound.
Hm. But what to do?
- rename all three functions to FOO_base(), and add a boolean parameter
- add FOO() wrappers, passing true (use generic error)
- add FOO_deprecated() wrappers, passing false (use device not found error)
- change existing callers to use FOO_deprecated()
Still, new generic-error-based blk_by_qdev_id() and qmp_get_blk() will be
unused..
That seem too ugly for me. And that doesn't give 100% sure that nobody will
simply duplicate call to _deprecated() function...
Maybe better don't care for now (and continue use device-not-found for new
APIs, that reuse find_device_state()), and just declare the deprecation for
ERROR_CLASS_DEVICE_NOT_FOUND? And drop it usage everywhere after 3-releases
deprecation cycle.
Or do we want deprecate everything except for generic-error, and deprecate
error/class field in qmp return value altogether?
--
Best regards,
Vladimir