The command-line creation test is using QemuOpts. Switch it to keyval,
since the emulator has some special needs and thus the last user of
user_creatable_add_opts will go away with the next patch.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
tests/check-qom-proplist.c | 74 ++++++++++++++++++++++++++------------
1 file changed, 52 insertions(+), 22 deletions(-)
diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
index 1b76581980..73472944a0 100644
--- a/tests/check-qom-proplist.c
+++ b/tests/check-qom-proplist.c
@@ -21,6 +21,9 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qobject.h"
#include "qom/object.h"
#include "qemu/module.h"
#include "qemu/option.h"
@@ -398,44 +401,71 @@ static void test_dummy_createlist(void)
object_unparent(OBJECT(dobj));
}
+static bool test_create_obj(QDict *qdict, Error **errp)
+{
+ Visitor *v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
+ Object *obj = user_creatable_add_type(TYPE_DUMMY, "dev0", qdict, v, errp);
+
+ visit_free(v);
+ object_unref(obj);
+ return !!obj;
+}
+
static void test_dummy_createcmdl(void)
{
- QemuOpts *opts;
+ QDict *qdict;
DummyObject *dobj;
Error *err = NULL;
- const char *params = TYPE_DUMMY \
- ",id=dev0," \
- "bv=yes,sv=Hiss hiss hiss,av=platypus";
+ bool help;
+ const char *params = "bv=yes,sv=Hiss hiss hiss,av=platypus";
+ /* Needed for user_creatable_del. */
qemu_add_opts(&qemu_object_opts);
- opts = qemu_opts_parse(&qemu_object_opts, params, true, &err);
+
+ qdict = keyval_parse(params, "qom-type", &help, &err);
g_assert(err == NULL);
- g_assert(opts);
+ g_assert(qdict);
+ g_assert(!help);
- dobj = DUMMY_OBJECT(user_creatable_add_opts(opts, &err));
+ g_assert(test_create_obj(qdict, &err));
g_assert(err == NULL);
+ qobject_unref(qdict);
+
+ dobj = DUMMY_OBJECT(object_resolve_path_component(object_get_objects_root(),
+ "dev0"));
g_assert(dobj);
g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss");
g_assert(dobj->bv == true);
g_assert(dobj->av == DUMMY_PLATYPUS);
+ qdict = keyval_parse(params, "qom-type", &help, &err);
+ g_assert(!test_create_obj(qdict, &err));
+ g_assert(err);
+ g_assert(object_resolve_path_component(object_get_objects_root(), "dev0")
+ == OBJECT(dobj));
+ qobject_unref(qdict);
+ error_free(err);
+ err = NULL;
+
+ qdict = keyval_parse(params, "qom-type", &help, &err);
user_creatable_del("dev0", &error_abort);
+ g_assert(object_resolve_path_component(object_get_objects_root(), "dev0")
+ == NULL);
- object_unref(OBJECT(dobj));
-
- /*
- * cmdline-parsing via qemu_opts_parse() results in a QemuOpts entry
- * corresponding to the Object's ID to be added to the QemuOptsList
- * for objects. To avoid having this entry conflict with future
- * Objects using the same ID (which can happen in cases where
- * qemu_opts_parse() is used to parse the object params, such as
- * with hmp_object_add() at the time of this comment), we need to
- * check for this in user_creatable_del() and remove the QemuOpts if
- * it is present.
- *
- * The below check ensures this works as expected.
- */
- g_assert_null(qemu_opts_find(&qemu_object_opts, "dev0"));
+ g_assert(test_create_obj(qdict, &err));
+ g_assert(err == NULL);
+ qobject_unref(qdict);
+
+ dobj = DUMMY_OBJECT(object_resolve_path_component(object_get_objects_root(),
+ "dev0"));
+ g_assert(dobj);
+ g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss");
+ g_assert(dobj->bv == true);
+ g_assert(dobj->av == DUMMY_PLATYPUS);
+ g_assert(object_resolve_path_component(object_get_objects_root(), "dev0")
+ == OBJECT(dobj));
+
+ object_unparent(OBJECT(dobj));
}
static void test_dummy_badenum(void)
--
2.26.2
On 3/11/21 11:24 AM, Paolo Bonzini wrote:
> The command-line creation test is using QemuOpts. Switch it to keyval,
> since the emulator has some special needs and thus the last user of
> user_creatable_add_opts will go away with the next patch.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> tests/check-qom-proplist.c | 74 ++++++++++++++++++++++++++------------
> 1 file changed, 52 insertions(+), 22 deletions(-)
>
> static void test_dummy_createcmdl(void)
> {
> - QemuOpts *opts;
> + QDict *qdict;
> DummyObject *dobj;
> Error *err = NULL;
> - const char *params = TYPE_DUMMY \
> - ",id=dev0," \
> - "bv=yes,sv=Hiss hiss hiss,av=platypus";
> + bool help;
> + const char *params = "bv=yes,sv=Hiss hiss hiss,av=platypus";
>
> + /* Needed for user_creatable_del. */
> qemu_add_opts(&qemu_object_opts);
> - opts = qemu_opts_parse(&qemu_object_opts, params, true, &err);
> +
> + qdict = keyval_parse(params, "qom-type", &help, &err);
> g_assert(err == NULL);
> - g_assert(opts);
> + g_assert(qdict);
> + g_assert(!help);
>
> - dobj = DUMMY_OBJECT(user_creatable_add_opts(opts, &err));
> + g_assert(test_create_obj(qdict, &err));
This performs a side-effect inside of g_assert(). Do we care? On the
one hand, this is a testsuite (where disabling g_assert weakens the
test), on the other hand, even if we can guarantee g_assert is not
disabled in the testsuite, it lends itself to poor copy-and-paste
practice to other sites. Better would be to assign to a bool outside
g_assert(), then assert the variable.
> g_assert(err == NULL);
> + qobject_unref(qdict);
> +
> + dobj = DUMMY_OBJECT(object_resolve_path_component(object_get_objects_root(),
> + "dev0"));
> g_assert(dobj);
> g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss");
> g_assert(dobj->bv == true);
> g_assert(dobj->av == DUMMY_PLATYPUS);
>
> + qdict = keyval_parse(params, "qom-type", &help, &err);
> + g_assert(!test_create_obj(qdict, &err));
And again.
> + g_assert(err);
> + g_assert(object_resolve_path_component(object_get_objects_root(), "dev0")
> + == OBJECT(dobj));
> + qobject_unref(qdict);
> + error_free(err);
> + err = NULL;
> +
> + qdict = keyval_parse(params, "qom-type", &help, &err);
> user_creatable_del("dev0", &error_abort);
> + g_assert(object_resolve_path_component(object_get_objects_root(), "dev0")
> + == NULL);
>
> - object_unref(OBJECT(dobj));
> -
> - /*
> - * cmdline-parsing via qemu_opts_parse() results in a QemuOpts entry
> - * corresponding to the Object's ID to be added to the QemuOptsList
> - * for objects. To avoid having this entry conflict with future
> - * Objects using the same ID (which can happen in cases where
> - * qemu_opts_parse() is used to parse the object params, such as
> - * with hmp_object_add() at the time of this comment), we need to
> - * check for this in user_creatable_del() and remove the QemuOpts if
> - * it is present.
> - *
> - * The below check ensures this works as expected.
> - */
> - g_assert_null(qemu_opts_find(&qemu_object_opts, "dev0"));
> + g_assert(test_create_obj(qdict, &err));
and again
> + g_assert(err == NULL);
> + qobject_unref(qdict);
> +
> + dobj = DUMMY_OBJECT(object_resolve_path_component(object_get_objects_root(),
> + "dev0"));
> + g_assert(dobj);
> + g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss");
> + g_assert(dobj->bv == true);
> + g_assert(dobj->av == DUMMY_PLATYPUS);
> + g_assert(object_resolve_path_component(object_get_objects_root(), "dev0")
> + == OBJECT(dobj));
> +
> + object_unparent(OBJECT(dobj));
> }
>
> static void test_dummy_badenum(void)
>
Otherwise, this looks like a sane thing to do.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Am 11.03.2021 um 18:24 hat Paolo Bonzini geschrieben: > The command-line creation test is using QemuOpts. Switch it to keyval, > since the emulator has some special needs and thus the last user of > user_creatable_add_opts will go away with the next patch. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
© 2016 - 2026 Red Hat, Inc.