include/qom/object.h | 24 ++++++++++++++++++++++-- qom/object.c | 33 ++++++++++++++++++++++++++++----- qom/object_interfaces.c | 7 +++++-- 3 files changed, 55 insertions(+), 9 deletions(-)
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:
For instance, 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 two new functions, object_property_add_err() and
object_property_add_child_err() whose prototype features an error handle.
object_property_add_child_err() now gets called from user_creatable_add_type.
This solution was chosen instead of changing the prototype of existing
functions because the number of existing callers is huge.
The error now 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>
---
include/qom/object.h | 24 ++++++++++++++++++++++--
qom/object.c | 33 ++++++++++++++++++++++++++++-----
qom/object_interfaces.c | 7 +++++--
3 files changed, 55 insertions(+), 9 deletions(-)
diff --git a/include/qom/object.h b/include/qom/object.h
index 94a61ccc3f..29652a1563 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1039,7 +1039,7 @@ Object *object_ref(Object *obj);
void object_unref(Object *obj);
/**
- * object_property_add:
+ * object_property_add_err:
* @obj: the object to add a property to
* @name: the name of the property. This can contain any character except for
* a forward slash. In general, you should use hyphens '-' instead of
@@ -1056,10 +1056,22 @@ void object_unref(Object *obj);
* meant to allow a property to free its opaque upon object
* destruction. This may be NULL.
* @opaque: an opaque pointer to pass to the callbacks for the property
+ * @errp: error handle
*
* Returns: The #ObjectProperty; this can be used to set the @resolve
* callback for child and link properties.
*/
+ObjectProperty *object_property_add_err(Object *obj, const char *name,
+ const char *type,
+ ObjectPropertyAccessor *get,
+ ObjectPropertyAccessor *set,
+ ObjectPropertyRelease *release,
+ void *opaque, Error **errp);
+
+/**
+ * object_property_add: same as object_property_add with
+ * errp hardcoded to &error_abort
+ */
ObjectProperty *object_property_add(Object *obj, const char *name,
const char *type,
ObjectPropertyAccessor *get,
@@ -1495,10 +1507,11 @@ Object *object_resolve_path_type(const char *path, const char *typename,
Object *object_resolve_path_component(Object *parent, const char *part);
/**
- * object_property_add_child:
+ * object_property_add_child_err:
* @obj: the object to add a property to
* @name: the name of the property
* @child: the child object
+ * @errp: error handle
*
* Child properties form the composition tree. All objects need to be a child
* of another object. Objects can only be a child of one object.
@@ -1512,6 +1525,13 @@ Object *object_resolve_path_component(Object *parent, const char *part);
*
* Returns: The newly added property on success, or %NULL on failure.
*/
+ObjectProperty *object_property_add_child_err(Object *obj, const char *name,
+ Object *child, Error **errp);
+
+/**
+ * object_property_add_child: same as object_property_add_child_err with
+ * errp hardcoded to &error_abort
+ */
ObjectProperty *object_property_add_child(Object *obj, const char *name,
Object *child);
diff --git a/qom/object.c b/qom/object.c
index 6ece96bc2b..2ec72fae7c 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1182,6 +1182,17 @@ object_property_try_add(Object *obj, const char *name, const char *type,
return prop;
}
+ObjectProperty *
+object_property_add_err(Object *obj, const char *name, const char *type,
+ ObjectPropertyAccessor *get,
+ ObjectPropertyAccessor *set,
+ ObjectPropertyRelease *release,
+ void *opaque, Error **errp)
+{
+ return object_property_try_add(obj, name, type, get, set, release,
+ opaque, errp);
+}
+
ObjectProperty *
object_property_add(Object *obj, const char *name, const char *type,
ObjectPropertyAccessor *get,
@@ -1189,7 +1200,7 @@ object_property_add(Object *obj, const char *name, const char *type,
ObjectPropertyRelease *release,
void *opaque)
{
- return object_property_try_add(obj, name, type, get, set, release,
+ return object_property_add_err(obj, name, type, get, set, release,
opaque, &error_abort);
}
@@ -1651,8 +1662,8 @@ static void object_finalize_child_property(Object *obj, const char *name,
}
ObjectProperty *
-object_property_add_child(Object *obj, const char *name,
- Object *child)
+object_property_add_child_err(Object *obj, const char *name,
+ Object *child, Error **errp)
{
g_autofree char *type = NULL;
ObjectProperty *op;
@@ -1661,14 +1672,26 @@ object_property_add_child(Object *obj, const char *name,
type = g_strdup_printf("child<%s>", object_get_typename(child));
- op = object_property_add(obj, name, type, object_get_child_property, NULL,
- object_finalize_child_property, child);
+ op = object_property_add_err(obj, name, type, object_get_child_property,
+ NULL, object_finalize_child_property,
+ child, errp);
+ if (!op) {
+ goto out;
+ }
op->resolve = object_resolve_child_property;
+out:
object_ref(child);
child->parent = obj;
return op;
}
+ObjectProperty *
+object_property_add_child(Object *obj, const char *name,
+ Object *child)
+{
+ return object_property_add_child_err(obj, name, child, &error_abort);
+}
+
void object_property_allow_set_link(const Object *obj, const char *name,
Object *val, Error **errp)
{
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 7e26f86fa6..c7fabda284 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -82,8 +82,11 @@ Object *user_creatable_add_type(const char *type, const char *id,
}
if (id != NULL) {
- object_property_add_child(object_get_objects_root(),
- id, obj);
+ object_property_add_child_err(object_get_objects_root(),
+ id, obj, &local_err);
+ if (local_err) {
+ goto out;
+ }
}
user_creatable_complete(USER_CREATABLE(obj), &local_err);
--
2.20.1
On 23/06/20 17:54, Eric Auger wrote:
> 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:
>
> For instance, 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 two new functions, object_property_add_err() and
> object_property_add_child_err() whose prototype features an error handle.
> object_property_add_child_err() now gets called from user_creatable_add_type.
> This solution was chosen instead of changing the prototype of existing
> functions because the number of existing callers is huge.
>
> The error now 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')"}}
... also, please add a testcase for this.
Paolo
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
> include/qom/object.h | 24 ++++++++++++++++++++++--
> qom/object.c | 33 ++++++++++++++++++++++++++++-----
> qom/object_interfaces.c | 7 +++++--
> 3 files changed, 55 insertions(+), 9 deletions(-)
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 94a61ccc3f..29652a1563 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -1039,7 +1039,7 @@ Object *object_ref(Object *obj);
> void object_unref(Object *obj);
>
> /**
> - * object_property_add:
> + * object_property_add_err:
> * @obj: the object to add a property to
> * @name: the name of the property. This can contain any character except for
> * a forward slash. In general, you should use hyphens '-' instead of
> @@ -1056,10 +1056,22 @@ void object_unref(Object *obj);
> * meant to allow a property to free its opaque upon object
> * destruction. This may be NULL.
> * @opaque: an opaque pointer to pass to the callbacks for the property
> + * @errp: error handle
> *
> * Returns: The #ObjectProperty; this can be used to set the @resolve
> * callback for child and link properties.
> */
> +ObjectProperty *object_property_add_err(Object *obj, const char *name,
> + const char *type,
> + ObjectPropertyAccessor *get,
> + ObjectPropertyAccessor *set,
> + ObjectPropertyRelease *release,
> + void *opaque, Error **errp);
> +
> +/**
> + * object_property_add: same as object_property_add with
> + * errp hardcoded to &error_abort
> + */
> ObjectProperty *object_property_add(Object *obj, const char *name,
> const char *type,
> ObjectPropertyAccessor *get,
> @@ -1495,10 +1507,11 @@ Object *object_resolve_path_type(const char *path, const char *typename,
> Object *object_resolve_path_component(Object *parent, const char *part);
>
> /**
> - * object_property_add_child:
> + * object_property_add_child_err:
> * @obj: the object to add a property to
> * @name: the name of the property
> * @child: the child object
> + * @errp: error handle
> *
> * Child properties form the composition tree. All objects need to be a child
> * of another object. Objects can only be a child of one object.
> @@ -1512,6 +1525,13 @@ Object *object_resolve_path_component(Object *parent, const char *part);
> *
> * Returns: The newly added property on success, or %NULL on failure.
> */
> +ObjectProperty *object_property_add_child_err(Object *obj, const char *name,
> + Object *child, Error **errp);
> +
> +/**
> + * object_property_add_child: same as object_property_add_child_err with
> + * errp hardcoded to &error_abort
> + */
> ObjectProperty *object_property_add_child(Object *obj, const char *name,
> Object *child);
>
> diff --git a/qom/object.c b/qom/object.c
> index 6ece96bc2b..2ec72fae7c 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1182,6 +1182,17 @@ object_property_try_add(Object *obj, const char *name, const char *type,
> return prop;
> }
>
> +ObjectProperty *
> +object_property_add_err(Object *obj, const char *name, const char *type,
> + ObjectPropertyAccessor *get,
> + ObjectPropertyAccessor *set,
> + ObjectPropertyRelease *release,
> + void *opaque, Error **errp)
> +{
> + return object_property_try_add(obj, name, type, get, set, release,
> + opaque, errp);
> +}
> +
> ObjectProperty *
> object_property_add(Object *obj, const char *name, const char *type,
> ObjectPropertyAccessor *get,
> @@ -1189,7 +1200,7 @@ object_property_add(Object *obj, const char *name, const char *type,
> ObjectPropertyRelease *release,
> void *opaque)
> {
> - return object_property_try_add(obj, name, type, get, set, release,
> + return object_property_add_err(obj, name, type, get, set, release,
> opaque, &error_abort);
> }
>
> @@ -1651,8 +1662,8 @@ static void object_finalize_child_property(Object *obj, const char *name,
> }
>
> ObjectProperty *
> -object_property_add_child(Object *obj, const char *name,
> - Object *child)
> +object_property_add_child_err(Object *obj, const char *name,
> + Object *child, Error **errp)
> {
> g_autofree char *type = NULL;
> ObjectProperty *op;
> @@ -1661,14 +1672,26 @@ object_property_add_child(Object *obj, const char *name,
>
> type = g_strdup_printf("child<%s>", object_get_typename(child));
>
> - op = object_property_add(obj, name, type, object_get_child_property, NULL,
> - object_finalize_child_property, child);
> + op = object_property_add_err(obj, name, type, object_get_child_property,
> + NULL, object_finalize_child_property,
> + child, errp);
> + if (!op) {
> + goto out;
> + }
> op->resolve = object_resolve_child_property;
> +out:
> object_ref(child);
> child->parent = obj;
> return op;
> }
>
> +ObjectProperty *
> +object_property_add_child(Object *obj, const char *name,
> + Object *child)
> +{
> + return object_property_add_child_err(obj, name, child, &error_abort);
> +}
> +
> void object_property_allow_set_link(const Object *obj, const char *name,
> Object *val, Error **errp)
> {
> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
> index 7e26f86fa6..c7fabda284 100644
> --- a/qom/object_interfaces.c
> +++ b/qom/object_interfaces.c
> @@ -82,8 +82,11 @@ Object *user_creatable_add_type(const char *type, const char *id,
> }
>
> if (id != NULL) {
> - object_property_add_child(object_get_objects_root(),
> - id, obj);
> + object_property_add_child_err(object_get_objects_root(),
> + id, obj, &local_err);
> + if (local_err) {
> + goto out;
> + }
> }
>
> user_creatable_complete(USER_CREATABLE(obj), &local_err);
>
Eric Auger <eric.auger@redhat.com> writes:
> 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:
>
> For instance, 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 two new functions, object_property_add_err() and
> object_property_add_child_err() whose prototype features an error handle.
> object_property_add_child_err() now gets called from user_creatable_add_type.
> This solution was chosen instead of changing the prototype of existing
> functions because the number of existing callers is huge.
>
> The error now 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>
Recent regression, my fault. Please point that out, and add
Fixes: d2623129a7dec1d3041ad1221dda1ca49c667532
On Wed, Jun 24, 2020 at 10:22:14AM +0200, Markus Armbruster wrote:
> Eric Auger <eric.auger@redhat.com> writes:
>
> > 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:
> >
> > For instance, 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 two new functions, object_property_add_err() and
> > object_property_add_child_err() whose prototype features an error handle.
> > object_property_add_child_err() now gets called from user_creatable_add_type.
> > This solution was chosen instead of changing the prototype of existing
> > functions because the number of existing callers is huge.
> >
> > The error now 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>
>
> Recent regression, my fault. Please point that out, and add
>
> Fixes: d2623129a7dec1d3041ad1221dda1ca49c667532
I noticed tests/qtest/qmp-cmd-test.c exercises object-add. Probably a
good idea to extend that to test duplicate ID scenario, as that would
have caught the accidental regression.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Hi Markus,
On 6/24/20 10:22 AM, Markus Armbruster wrote:
> Eric Auger <eric.auger@redhat.com> writes:
>
>> 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:
>>
>> For instance, 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 two new functions, object_property_add_err() and
>> object_property_add_child_err() whose prototype features an error handle.
>> object_property_add_child_err() now gets called from user_creatable_add_type.
>> This solution was chosen instead of changing the prototype of existing
>> functions because the number of existing callers is huge.
>>
>> The error now 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>
>
> Recent regression, my fault. Please point that out, and add
>
> Fixes: d2623129a7dec1d3041ad1221dda1ca49c667532
Thanks you for the reference. I will add the tag.
Eric
>
On 23/06/20 17:54, Eric Auger wrote:
> This patch introduces two new functions, object_property_add_err() and
> object_property_add_child_err() whose prototype features an error handle.
> object_property_add_child_err() now gets called from user_creatable_add_type.
> This solution was chosen instead of changing the prototype of existing
> functions because the number of existing callers is huge.
The idea is good, but I would rather name the functions
object_property_try_add{,_child} to follow e.g. g_try_malloc.
In fact, the existing function object_property_try_add can simply be
made non-static.
Paolo
Hi Paolo,
On 6/23/20 6:22 PM, Paolo Bonzini wrote:
> On 23/06/20 17:54, Eric Auger wrote:
>> This patch introduces two new functions, object_property_add_err() and
>> object_property_add_child_err() whose prototype features an error handle.
>> object_property_add_child_err() now gets called from user_creatable_add_type.
>> This solution was chosen instead of changing the prototype of existing
>> functions because the number of existing callers is huge.
>
> The idea is good, but I would rather name the functions
> object_property_try_add{,_child} to follow e.g. g_try_malloc.
>
> In fact, the existing function object_property_try_add can simply be
> made non-static.
Sure I will respin accordingly
Thanks
Eric
>
> Paolo
>
On 23/06/20 17:54, Eric Auger wrote:
> 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:
>
> For instance, 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 two new functions, object_property_add_err() and
> object_property_add_child_err() whose prototype features an error handle.
> object_property_add_child_err() now gets called from user_creatable_add_type.
> This solution was chosen instead of changing the prototype of existing
> functions because the number of existing callers is huge.
>
> The error now 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>
> ---
> include/qom/object.h | 24 ++++++++++++++++++++++--
> qom/object.c | 33 ++++++++++++++++++++++++++++-----
> qom/object_interfaces.c | 7 +++++--
> 3 files changed, 55 insertions(+), 9 deletions(-)
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 94a61ccc3f..29652a1563 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -1039,7 +1039,7 @@ Object *object_ref(Object *obj);
> void object_unref(Object *obj);
>
> /**
> - * object_property_add:
> + * object_property_add_err:
> * @obj: the object to add a property to
> * @name: the name of the property. This can contain any character except for
> * a forward slash. In general, you should use hyphens '-' instead of
> @@ -1056,10 +1056,22 @@ void object_unref(Object *obj);
> * meant to allow a property to free its opaque upon object
> * destruction. This may be NULL.
> * @opaque: an opaque pointer to pass to the callbacks for the property
> + * @errp: error handle
> *
> * Returns: The #ObjectProperty; this can be used to set the @resolve
> * callback for child and link properties.
> */
> +ObjectProperty *object_property_add_err(Object *obj, const char *name,
> + const char *type,
> + ObjectPropertyAccessor *get,
> + ObjectPropertyAccessor *set,
> + ObjectPropertyRelease *release,
> + void *opaque, Error **errp);
> +
> +/**
> + * object_property_add: same as object_property_add with
> + * errp hardcoded to &error_abort
> + */
> ObjectProperty *object_property_add(Object *obj, const char *name,
> const char *type,
> ObjectPropertyAccessor *get,
> @@ -1495,10 +1507,11 @@ Object *object_resolve_path_type(const char *path, const char *typename,
> Object *object_resolve_path_component(Object *parent, const char *part);
>
> /**
> - * object_property_add_child:
> + * object_property_add_child_err:
> * @obj: the object to add a property to
> * @name: the name of the property
> * @child: the child object
> + * @errp: error handle
> *
> * Child properties form the composition tree. All objects need to be a child
> * of another object. Objects can only be a child of one object.
> @@ -1512,6 +1525,13 @@ Object *object_resolve_path_component(Object *parent, const char *part);
> *
> * Returns: The newly added property on success, or %NULL on failure.
> */
> +ObjectProperty *object_property_add_child_err(Object *obj, const char *name,
> + Object *child, Error **errp);
> +
> +/**
> + * object_property_add_child: same as object_property_add_child_err with
> + * errp hardcoded to &error_abort
> + */
> ObjectProperty *object_property_add_child(Object *obj, const char *name,
> Object *child);
>
> diff --git a/qom/object.c b/qom/object.c
> index 6ece96bc2b..2ec72fae7c 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1182,6 +1182,17 @@ object_property_try_add(Object *obj, const char *name, const char *type,
> return prop;
> }
>
> +ObjectProperty *
> +object_property_add_err(Object *obj, const char *name, const char *type,
> + ObjectPropertyAccessor *get,
> + ObjectPropertyAccessor *set,
> + ObjectPropertyRelease *release,
> + void *opaque, Error **errp)
> +{
> + return object_property_try_add(obj, name, type, get, set, release,
> + opaque, errp);
> +}
> +
> ObjectProperty *
> object_property_add(Object *obj, const char *name, const char *type,
> ObjectPropertyAccessor *get,
> @@ -1189,7 +1200,7 @@ object_property_add(Object *obj, const char *name, const char *type,
> ObjectPropertyRelease *release,
> void *opaque)
> {
> - return object_property_try_add(obj, name, type, get, set, release,
> + return object_property_add_err(obj, name, type, get, set, release,
> opaque, &error_abort);
> }
>
> @@ -1651,8 +1662,8 @@ static void object_finalize_child_property(Object *obj, const char *name,
> }
>
> ObjectProperty *
> -object_property_add_child(Object *obj, const char *name,
> - Object *child)
> +object_property_add_child_err(Object *obj, const char *name,
> + Object *child, Error **errp)
> {
> g_autofree char *type = NULL;
> ObjectProperty *op;
> @@ -1661,14 +1672,26 @@ object_property_add_child(Object *obj, const char *name,
>
> type = g_strdup_printf("child<%s>", object_get_typename(child));
>
> - op = object_property_add(obj, name, type, object_get_child_property, NULL,
> - object_finalize_child_property, child);
> + op = object_property_add_err(obj, name, type, object_get_child_property,
> + NULL, object_finalize_child_property,
> + child, errp);
> + if (!op) {
> + goto out;
> + }
> op->resolve = object_resolve_child_property;
> +out:
> object_ref(child);
> child->parent = obj;
> return op;
> }
>
> +ObjectProperty *
> +object_property_add_child(Object *obj, const char *name,
> + Object *child)
> +{
> + return object_property_add_child_err(obj, name, child, &error_abort);
> +}
> +
> void object_property_allow_set_link(const Object *obj, const char *name,
> Object *val, Error **errp)
> {
> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
> index 7e26f86fa6..c7fabda284 100644
> --- a/qom/object_interfaces.c
> +++ b/qom/object_interfaces.c
> @@ -82,8 +82,11 @@ Object *user_creatable_add_type(const char *type, const char *id,
> }
>
> if (id != NULL) {
> - object_property_add_child(object_get_objects_root(),
> - id, obj);
> + object_property_add_child_err(object_get_objects_root(),
> + id, obj, &local_err);
> + if (local_err) {
> + goto out;
> + }
> }
>
> user_creatable_complete(USER_CREATABLE(obj), &local_err);
>
Queued, thanks.
Paolo
On Tue, Jun 23, 2020 at 05:54:52PM +0200, Eric Auger wrote:
> 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:
>
> For instance, 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 two new functions, object_property_add_err() and
> object_property_add_child_err() whose prototype features an error handle.
> object_property_add_child_err() now gets called from user_creatable_add_type.
> This solution was chosen instead of changing the prototype of existing
> functions because the number of existing callers is huge.
>
> The error now 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>
> ---
> include/qom/object.h | 24 ++++++++++++++++++++++--
> qom/object.c | 33 ++++++++++++++++++++++++++++-----
> qom/object_interfaces.c | 7 +++++--
> 3 files changed, 55 insertions(+), 9 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
© 2016 - 2024 Red Hat, Inc.