* [PATCH v5 1/3] qom: Introduce object_property_try_add_child()
2020-06-29 19:34 [PATCH v5 0/3] Avoid abort on QMP attempt to add an object with duplicate id Eric Auger
@ 2020-06-29 19:34 ` Eric Auger
2020-06-30 13:41 ` Greg Kurz
2020-06-29 19:34 ` [PATCH v5 2/3] tests/qmp-cmd-test: Add qmp/object-add-duplicate-id Eric Auger
2020-06-29 19:34 ` [PATCH v5 3/3] tests/qmp-cmd-test: Add qmp/object-add-failure-modes Eric Auger
2 siblings, 1 reply; 6+ messages in thread
From: Eric Auger @ 2020-06-29 19:34 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, qemu-devel, pbonzini, berrange,
ehabkost, armbru
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>
---
v3 -> v4:
- Took into account Markus' style related comments
v2 -> v3:
- don't take the object reference on failure in
object_property_try_add_child
---
include/qom/object.h | 26 ++++++++++++++++++++++++--
qom/object.c | 21 ++++++++++++++++-----
qom/object_interfaces.c | 7 +++++--
3 files changed, 45 insertions(+), 9 deletions(-)
diff --git a/include/qom/object.h b/include/qom/object.h
index 94a61ccc3f..1c5cdcd0e3 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_try_add:
* @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,23 @@ 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: pointer to error object
*
* Returns: The #ObjectProperty; this can be used to set the @resolve
* callback for child and link properties.
*/
+ObjectProperty *object_property_try_add(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_try_add() with @errp hardcoded to
+ * &error_abort.
+ */
ObjectProperty *object_property_add(Object *obj, const char *name,
const char *type,
ObjectPropertyAccessor *get,
@@ -1495,10 +1508,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_try_add_child:
* @obj: the object to add a property to
* @name: the name of the property
* @child: the child object
+ * @errp: pointer to error object
*
* 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 +1526,14 @@ Object *object_resolve_path_component(Object *parent, const char *part);
*
* Returns: The newly added property on success, or %NULL on failure.
*/
+ObjectProperty *object_property_try_add_child(Object *obj, const char *name,
+ Object *child, Error **errp);
+
+/**
+ * object_property_add_child:
+ * Same as object_property_try_add_child() 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..dc10bb1889 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1132,7 +1132,7 @@ void object_unref(Object *obj)
}
}
-static ObjectProperty *
+ObjectProperty *
object_property_try_add(Object *obj, const char *name, const char *type,
ObjectPropertyAccessor *get,
ObjectPropertyAccessor *set,
@@ -1651,8 +1651,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_try_add_child(Object *obj, const char *name,
+ Object *child, Error **errp)
{
g_autofree char *type = NULL;
ObjectProperty *op;
@@ -1661,14 +1661,25 @@ 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_try_add(obj, name, type, object_get_child_property,
+ NULL, object_finalize_child_property,
+ child, errp);
+ if (!op) {
+ return NULL;
+ }
op->resolve = object_resolve_child_property;
object_ref(child);
child->parent = obj;
return op;
}
+ObjectProperty *
+object_property_add_child(Object *obj, const char *name,
+ Object *child)
+{
+ return object_property_try_add_child(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..1e05e41d2f 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_try_add_child(object_get_objects_root(),
+ id, obj, &local_err);
+ if (local_err) {
+ goto out;
+ }
}
user_creatable_complete(USER_CREATABLE(obj), &local_err);
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v5 1/3] qom: Introduce object_property_try_add_child()
2020-06-29 19:34 ` [PATCH v5 1/3] qom: Introduce object_property_try_add_child() Eric Auger
@ 2020-06-30 13:41 ` Greg Kurz
2020-07-01 13:23 ` Auger Eric
0 siblings, 1 reply; 6+ messages in thread
From: Greg Kurz @ 2020-06-30 13:41 UTC (permalink / raw)
To: Eric Auger
Cc: berrange, ehabkost, armbru, qemu-devel, pbonzini, eric.auger.pro
On Mon, 29 Jun 2020 21:34:22 +0200
Eric Auger <eric.auger@redhat.com> 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. 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>
>
FWIW
Reviewed-by: Greg Kurz <groug@kaod.org>
and
Tested-by: Greg Kurz <groug@kaod.org>
> ---
>
> v3 -> v4:
> - Took into account Markus' style related comments
>
> v2 -> v3:
> - don't take the object reference on failure in
> object_property_try_add_child
> ---
> include/qom/object.h | 26 ++++++++++++++++++++++++--
> qom/object.c | 21 ++++++++++++++++-----
> qom/object_interfaces.c | 7 +++++--
> 3 files changed, 45 insertions(+), 9 deletions(-)
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 94a61ccc3f..1c5cdcd0e3 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_try_add:
> * @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,23 @@ 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: pointer to error object
> *
> * Returns: The #ObjectProperty; this can be used to set the @resolve
> * callback for child and link properties.
> */
> +ObjectProperty *object_property_try_add(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_try_add() with @errp hardcoded to
> + * &error_abort.
> + */
> ObjectProperty *object_property_add(Object *obj, const char *name,
> const char *type,
> ObjectPropertyAccessor *get,
> @@ -1495,10 +1508,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_try_add_child:
> * @obj: the object to add a property to
> * @name: the name of the property
> * @child: the child object
> + * @errp: pointer to error object
> *
> * 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 +1526,14 @@ Object *object_resolve_path_component(Object *parent, const char *part);
> *
> * Returns: The newly added property on success, or %NULL on failure.
> */
> +ObjectProperty *object_property_try_add_child(Object *obj, const char *name,
> + Object *child, Error **errp);
> +
> +/**
> + * object_property_add_child:
> + * Same as object_property_try_add_child() 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..dc10bb1889 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1132,7 +1132,7 @@ void object_unref(Object *obj)
> }
> }
>
> -static ObjectProperty *
> +ObjectProperty *
> object_property_try_add(Object *obj, const char *name, const char *type,
> ObjectPropertyAccessor *get,
> ObjectPropertyAccessor *set,
> @@ -1651,8 +1651,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_try_add_child(Object *obj, const char *name,
> + Object *child, Error **errp)
> {
> g_autofree char *type = NULL;
> ObjectProperty *op;
> @@ -1661,14 +1661,25 @@ 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_try_add(obj, name, type, object_get_child_property,
> + NULL, object_finalize_child_property,
> + child, errp);
> + if (!op) {
> + return NULL;
> + }
> op->resolve = object_resolve_child_property;
> object_ref(child);
> child->parent = obj;
> return op;
> }
>
> +ObjectProperty *
> +object_property_add_child(Object *obj, const char *name,
> + Object *child)
> +{
> + return object_property_try_add_child(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..1e05e41d2f 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_try_add_child(object_get_objects_root(),
> + id, obj, &local_err);
> + if (local_err) {
> + goto out;
> + }
> }
>
> user_creatable_complete(USER_CREATABLE(obj), &local_err);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v5 1/3] qom: Introduce object_property_try_add_child()
2020-06-30 13:41 ` Greg Kurz
@ 2020-07-01 13:23 ` Auger Eric
0 siblings, 0 replies; 6+ messages in thread
From: Auger Eric @ 2020-07-01 13:23 UTC (permalink / raw)
To: Greg Kurz
Cc: berrange, ehabkost, armbru, qemu-devel, pbonzini, eric.auger.pro
Hi Greg,
On 6/30/20 3:41 PM, Greg Kurz wrote:
> On Mon, 29 Jun 2020 21:34:22 +0200
> Eric Auger <eric.auger@redhat.com> 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. 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>
>>
>
> FWIW
>
> Reviewed-by: Greg Kurz <groug@kaod.org>
>
> and
>
> Tested-by: Greg Kurz <groug@kaod.org>
Thanks!
Eric
>
>> ---
>>
>> v3 -> v4:
>> - Took into account Markus' style related comments
>>
>> v2 -> v3:
>> - don't take the object reference on failure in
>> object_property_try_add_child
>> ---
>> include/qom/object.h | 26 ++++++++++++++++++++++++--
>> qom/object.c | 21 ++++++++++++++++-----
>> qom/object_interfaces.c | 7 +++++--
>> 3 files changed, 45 insertions(+), 9 deletions(-)
>>
>> diff --git a/include/qom/object.h b/include/qom/object.h
>> index 94a61ccc3f..1c5cdcd0e3 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_try_add:
>> * @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,23 @@ 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: pointer to error object
>> *
>> * Returns: The #ObjectProperty; this can be used to set the @resolve
>> * callback for child and link properties.
>> */
>> +ObjectProperty *object_property_try_add(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_try_add() with @errp hardcoded to
>> + * &error_abort.
>> + */
>> ObjectProperty *object_property_add(Object *obj, const char *name,
>> const char *type,
>> ObjectPropertyAccessor *get,
>> @@ -1495,10 +1508,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_try_add_child:
>> * @obj: the object to add a property to
>> * @name: the name of the property
>> * @child: the child object
>> + * @errp: pointer to error object
>> *
>> * 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 +1526,14 @@ Object *object_resolve_path_component(Object *parent, const char *part);
>> *
>> * Returns: The newly added property on success, or %NULL on failure.
>> */
>> +ObjectProperty *object_property_try_add_child(Object *obj, const char *name,
>> + Object *child, Error **errp);
>> +
>> +/**
>> + * object_property_add_child:
>> + * Same as object_property_try_add_child() 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..dc10bb1889 100644
>> --- a/qom/object.c
>> +++ b/qom/object.c
>> @@ -1132,7 +1132,7 @@ void object_unref(Object *obj)
>> }
>> }
>>
>> -static ObjectProperty *
>> +ObjectProperty *
>> object_property_try_add(Object *obj, const char *name, const char *type,
>> ObjectPropertyAccessor *get,
>> ObjectPropertyAccessor *set,
>> @@ -1651,8 +1651,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_try_add_child(Object *obj, const char *name,
>> + Object *child, Error **errp)
>> {
>> g_autofree char *type = NULL;
>> ObjectProperty *op;
>> @@ -1661,14 +1661,25 @@ 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_try_add(obj, name, type, object_get_child_property,
>> + NULL, object_finalize_child_property,
>> + child, errp);
>> + if (!op) {
>> + return NULL;
>> + }
>> op->resolve = object_resolve_child_property;
>> object_ref(child);
>> child->parent = obj;
>> return op;
>> }
>>
>> +ObjectProperty *
>> +object_property_add_child(Object *obj, const char *name,
>> + Object *child)
>> +{
>> + return object_property_try_add_child(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..1e05e41d2f 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_try_add_child(object_get_objects_root(),
>> + id, obj, &local_err);
>> + if (local_err) {
>> + goto out;
>> + }
>> }
>>
>> user_creatable_complete(USER_CREATABLE(obj), &local_err);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v5 2/3] tests/qmp-cmd-test: Add qmp/object-add-duplicate-id
2020-06-29 19:34 [PATCH v5 0/3] Avoid abort on QMP attempt to add an object with duplicate id Eric Auger
2020-06-29 19:34 ` [PATCH v5 1/3] qom: Introduce object_property_try_add_child() Eric Auger
@ 2020-06-29 19:34 ` Eric Auger
2020-06-29 19:34 ` [PATCH v5 3/3] tests/qmp-cmd-test: Add qmp/object-add-failure-modes Eric Auger
2 siblings, 0 replies; 6+ messages in thread
From: Eric Auger @ 2020-06-29 19:34 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, qemu-devel, pbonzini, berrange,
ehabkost, armbru
This new test checks that attempting to create an object
with an existing ID gracefully fails.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Acked-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
v4 -> v5
- use 1MB instead of 4GB
---
tests/qtest/qmp-cmd-test.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
index 9f5228cd99..fc65fa3726 100644
--- a/tests/qtest/qmp-cmd-test.c
+++ b/tests/qtest/qmp-cmd-test.c
@@ -213,6 +213,23 @@ static void test_object_add_without_props(void)
qtest_quit(qts);
}
+static void test_object_add_with_duplicate_id(void)
+{
+ QTestState *qts;
+ QDict *resp;
+
+ qts = qtest_init(common_args);
+ resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
+ " {'qom-type': 'memory-backend-ram', 'id': 'ram1', 'props': {'size': 1048576 } } }");
+ g_assert_nonnull(resp);
+ g_assert(qdict_haskey(resp, "return"));
+ resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
+ " {'qom-type': 'memory-backend-ram', 'id': 'ram1', 'props': {'size': 1048576 } } }");
+ g_assert_nonnull(resp);
+ qmp_assert_error_class(resp, "GenericError");
+ qtest_quit(qts);
+}
+
int main(int argc, char *argv[])
{
QmpSchema schema;
@@ -225,6 +242,8 @@ int main(int argc, char *argv[])
qtest_add_func("qmp/object-add-without-props",
test_object_add_without_props);
+ qtest_add_func("qmp/object-add-duplicate-id",
+ test_object_add_with_duplicate_id);
/* TODO: add coverage of generic object-add failure modes */
ret = g_test_run();
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v5 3/3] tests/qmp-cmd-test: Add qmp/object-add-failure-modes
2020-06-29 19:34 [PATCH v5 0/3] Avoid abort on QMP attempt to add an object with duplicate id Eric Auger
2020-06-29 19:34 ` [PATCH v5 1/3] qom: Introduce object_property_try_add_child() Eric Auger
2020-06-29 19:34 ` [PATCH v5 2/3] tests/qmp-cmd-test: Add qmp/object-add-duplicate-id Eric Auger
@ 2020-06-29 19:34 ` Eric Auger
2 siblings, 0 replies; 6+ messages in thread
From: Eric Auger @ 2020-06-29 19:34 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, qemu-devel, pbonzini, berrange,
ehabkost, armbru
Merge the existing object-add test cases into a single test
functions and cover more failure cases.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v4 -> v5:
- use 1M instead of 4G
---
tests/qtest/qmp-cmd-test.c | 114 +++++++++++++++++++++++++++++++------
1 file changed, 97 insertions(+), 17 deletions(-)
diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
index fc65fa3726..c68f99f659 100644
--- a/tests/qtest/qmp-cmd-test.c
+++ b/tests/qtest/qmp-cmd-test.c
@@ -200,33 +200,116 @@ static void add_query_tests(QmpSchema *schema)
}
}
-static void test_object_add_without_props(void)
+static void test_object_add_failure_modes(void)
{
QTestState *qts;
QDict *resp;
+ /* attempt to create an object without props */
qts = qtest_init(common_args);
resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
- " {'qom-type': 'memory-backend-ram', 'id': 'ram1' } }");
+ " {'qom-type': 'memory-backend-ram', 'id': 'ram1' } }");
g_assert_nonnull(resp);
qmp_assert_error_class(resp, "GenericError");
- qtest_quit(qts);
-}
-static void test_object_add_with_duplicate_id(void)
-{
- QTestState *qts;
- QDict *resp;
-
- qts = qtest_init(common_args);
+ /* attempt to create an object without qom-type */
resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
- " {'qom-type': 'memory-backend-ram', 'id': 'ram1', 'props': {'size': 1048576 } } }");
+ " {'id': 'ram1' } }");
+ g_assert_nonnull(resp);
+ qmp_assert_error_class(resp, "GenericError");
+
+ /* attempt to delete an object that does not exist */
+ resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
+ " {'id': 'ram1' } }");
+ g_assert_nonnull(resp);
+ qmp_assert_error_class(resp, "GenericError");
+
+ /* attempt to create 2 objects with duplicate id */
+ resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
+ " {'qom-type': 'memory-backend-ram', 'id': 'ram1',"
+ " 'props': {'size': 1048576 } } }");
g_assert_nonnull(resp);
g_assert(qdict_haskey(resp, "return"));
resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
- " {'qom-type': 'memory-backend-ram', 'id': 'ram1', 'props': {'size': 1048576 } } }");
+ " {'qom-type': 'memory-backend-ram', 'id': 'ram1',"
+ " 'props': {'size': 1048576 } } }");
g_assert_nonnull(resp);
qmp_assert_error_class(resp, "GenericError");
+
+ /* delete ram1 object */
+ resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
+ " {'id': 'ram1' } }");
+ g_assert_nonnull(resp);
+ g_assert(qdict_haskey(resp, "return"));
+
+ /* attempt to create an object with a property of a wrong type */
+ resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
+ " {'qom-type': 'memory-backend-ram', 'id': 'ram1',"
+ " 'props': {'size': '1048576' } } }");
+ g_assert_nonnull(resp);
+ /* now do it right */
+ qmp_assert_error_class(resp, "GenericError");
+ resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
+ " {'qom-type': 'memory-backend-ram', 'id': 'ram1',"
+ " 'props': {'size': 1048576 } } }");
+ g_assert_nonnull(resp);
+ g_assert(qdict_haskey(resp, "return"));
+
+ /* delete ram1 object */
+ resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
+ " {'id': 'ram1' } }");
+ g_assert_nonnull(resp);
+ g_assert(qdict_haskey(resp, "return"));
+
+ /* attempt to create an object without the id */
+ resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
+ " {'qom-type': 'memory-backend-ram',"
+ " 'props': {'size': 1048576 } } }");
+ g_assert_nonnull(resp);
+ qmp_assert_error_class(resp, "GenericError");
+ /* now do it right */
+ resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
+ " {'qom-type': 'memory-backend-ram', 'id': 'ram1',"
+ " 'props': {'size': 1048576 } } }");
+ g_assert_nonnull(resp);
+ g_assert(qdict_haskey(resp, "return"));
+
+ /* delete ram1 object */
+ resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
+ " {'id': 'ram1' } }");
+ g_assert_nonnull(resp);
+ g_assert(qdict_haskey(resp, "return"));
+
+ /* attempt to set a non existing property */
+ resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
+ " {'qom-type': 'memory-backend-ram', 'id': 'ram1',"
+ " 'props': {'sized': 1048576 } } }");
+ g_assert_nonnull(resp);
+ qmp_assert_error_class(resp, "GenericError");
+ /* now do it right */
+ resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
+ " {'qom-type': 'memory-backend-ram', 'id': 'ram1',"
+ " 'props': {'size': 1048576 } } }");
+ g_assert_nonnull(resp);
+ g_assert(qdict_haskey(resp, "return"));
+
+ /* delete ram1 object without id */
+ resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
+ " {'ida': 'ram1' } }");
+ g_assert_nonnull(resp);
+
+ /* delete ram1 object */
+ resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
+ " {'id': 'ram1' } }");
+ g_assert_nonnull(resp);
+ g_assert(qdict_haskey(resp, "return"));
+
+ /* delete ram1 object that does not exist anymore*/
+ resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
+ " {'id': 'ram1' } }");
+ g_assert_nonnull(resp);
+ qmp_assert_error_class(resp, "GenericError");
+
qtest_quit(qts);
}
@@ -240,11 +323,8 @@ int main(int argc, char *argv[])
qmp_schema_init(&schema);
add_query_tests(&schema);
- qtest_add_func("qmp/object-add-without-props",
- test_object_add_without_props);
- qtest_add_func("qmp/object-add-duplicate-id",
- test_object_add_with_duplicate_id);
- /* TODO: add coverage of generic object-add failure modes */
+ qtest_add_func("qmp/object-add-failure-modes",
+ test_object_add_failure_modes);
ret = g_test_run();
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread