jcs.org / qemu
qemu with hax to log dma reads & writes jcs.org/2018/11/12/vfio
fork atom

qom: Introduce object_property_try_add_child()

object_property_add() does not allow object_property_try_add()
to gracefully fail as &error_abort is passed as an error handle.

However such failure can easily be triggered from the QMP shell when,
for instance, one attempts to create an object with an id that already
exists. This is achieved from the following call path:

qmp_object_add -> user_creatable_add_dict -> user_creatable_add_type ->
object_property_add_child -> object_property_add

For instance, from the qmp-shell, call twice:
object-add qom-type=memory-backend-ram id=mem1 props.size=1073741824
and QEMU aborts.

This behavior is undesired as a user/management application mistake
in reusing a property ID shouldn't result in loss of the VM and live
data within.

This patch introduces a new function, object_property_try_add_child()
which takes an error handle and turn object_property_try_add() into
a non-static one.

Now the call path becomes:

user_creatable_add_type -> object_property_try_add_child ->
object_property_try_add

and the error is returned gracefully to the QMP client.

(QEMU) object-add qom-type=memory-backend-ram id=mem2 props.size=4294967296
{"return": {}}
(QEMU) object-add qom-type=memory-backend-ram id=mem2 props.size=4294967296
{"error": {"class": "GenericError", "desc": "attempt to add duplicate property
'mem2' to object (type 'container')"}}

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Fixes: d2623129a7de ("qom: Drop parameter @errp of object_property_add() & friends")
Reviewed-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Greg Kurz <groug@kaod.org>
Tested-by: Greg Kurz <groug@kaod.org>
Message-Id: <20200629193424.30280-2-eric.auger@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

authored by

Eric Auger and committed by
Paolo Bonzini db57fef1 2880ffb0

+45 -9
+24 -2
include/qom/object.h
··· 1047 1047 void object_unref(Object *obj); 1048 1048 1049 1049 /** 1050 - * object_property_add: 1050 + * object_property_try_add: 1051 1051 * @obj: the object to add a property to 1052 1052 * @name: the name of the property. This can contain any character except for 1053 1053 * a forward slash. In general, you should use hyphens '-' instead of ··· 1064 1064 * meant to allow a property to free its opaque upon object 1065 1065 * destruction. This may be NULL. 1066 1066 * @opaque: an opaque pointer to pass to the callbacks for the property 1067 + * @errp: pointer to error object 1067 1068 * 1068 1069 * Returns: The #ObjectProperty; this can be used to set the @resolve 1069 1070 * callback for child and link properties. 1071 + */ 1072 + ObjectProperty *object_property_try_add(Object *obj, const char *name, 1073 + const char *type, 1074 + ObjectPropertyAccessor *get, 1075 + ObjectPropertyAccessor *set, 1076 + ObjectPropertyRelease *release, 1077 + void *opaque, Error **errp); 1078 + 1079 + /** 1080 + * object_property_add: 1081 + * Same as object_property_try_add() with @errp hardcoded to 1082 + * &error_abort. 1070 1083 */ 1071 1084 ObjectProperty *object_property_add(Object *obj, const char *name, 1072 1085 const char *type, ··· 1518 1531 Object *object_resolve_path_component(Object *parent, const char *part); 1519 1532 1520 1533 /** 1521 - * object_property_add_child: 1534 + * object_property_try_add_child: 1522 1535 * @obj: the object to add a property to 1523 1536 * @name: the name of the property 1524 1537 * @child: the child object 1538 + * @errp: pointer to error object 1525 1539 * 1526 1540 * Child properties form the composition tree. All objects need to be a child 1527 1541 * of another object. Objects can only be a child of one object. ··· 1534 1548 * The child object itself can be retrieved using object_property_get_link(). 1535 1549 * 1536 1550 * Returns: The newly added property on success, or %NULL on failure. 1551 + */ 1552 + ObjectProperty *object_property_try_add_child(Object *obj, const char *name, 1553 + Object *child, Error **errp); 1554 + 1555 + /** 1556 + * object_property_add_child: 1557 + * Same as object_property_try_add_child() with @errp hardcoded to 1558 + * &error_abort 1537 1559 */ 1538 1560 ObjectProperty *object_property_add_child(Object *obj, const char *name, 1539 1561 Object *child);
+16 -5
qom/object.c
··· 1146 1146 } 1147 1147 } 1148 1148 1149 - static ObjectProperty * 1149 + ObjectProperty * 1150 1150 object_property_try_add(Object *obj, const char *name, const char *type, 1151 1151 ObjectPropertyAccessor *get, 1152 1152 ObjectPropertyAccessor *set, ··· 1675 1675 } 1676 1676 1677 1677 ObjectProperty * 1678 - object_property_add_child(Object *obj, const char *name, 1679 - Object *child) 1678 + object_property_try_add_child(Object *obj, const char *name, 1679 + Object *child, Error **errp) 1680 1680 { 1681 1681 g_autofree char *type = NULL; 1682 1682 ObjectProperty *op; ··· 1685 1685 1686 1686 type = g_strdup_printf("child<%s>", object_get_typename(child)); 1687 1687 1688 - op = object_property_add(obj, name, type, object_get_child_property, NULL, 1689 - object_finalize_child_property, child); 1688 + op = object_property_try_add(obj, name, type, object_get_child_property, 1689 + NULL, object_finalize_child_property, 1690 + child, errp); 1691 + if (!op) { 1692 + return NULL; 1693 + } 1690 1694 op->resolve = object_resolve_child_property; 1691 1695 object_ref(child); 1692 1696 child->parent = obj; 1693 1697 return op; 1698 + } 1699 + 1700 + ObjectProperty * 1701 + object_property_add_child(Object *obj, const char *name, 1702 + Object *child) 1703 + { 1704 + return object_property_try_add_child(obj, name, child, &error_abort); 1694 1705 } 1695 1706 1696 1707 void object_property_allow_set_link(const Object *obj, const char *name,
+5 -2
qom/object_interfaces.c
··· 83 83 } 84 84 85 85 if (id != NULL) { 86 - object_property_add_child(object_get_objects_root(), 87 - id, obj); 86 + object_property_try_add_child(object_get_objects_root(), 87 + id, obj, &local_err); 88 + if (local_err) { 89 + goto out; 90 + } 88 91 } 89 92 90 93 if (!user_creatable_complete(USER_CREATABLE(obj), &local_err)) {