Paolo Bonzini <pbonz...@redhat.com> writes:
> The command-line creation test is using QemuOpts. Switch it to keyval,
> since all the -object command line options will follow
> qemu-storage-daemon and do the same.
>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
$ gdb tests/check-qom-proplist
[...]
(gdb) r
[...]
# random seed: R02S802948119789b5481f33f9842e3b5d1b
1..9
# Start of qom tests
# Start of proplist tests
ok 1 /qom/proplist/createlist
ok 2 /qom/proplist/createv
Unexpected error in find_list() at ../util/qemu-config.c:24:
There is no option group 'object'
Thread 1 "check-qom-propl" received signal SIGABRT, Aborted.
0x00007ffff7b839e5 in raise () from /lib64/libc.so.6
Missing separate debuginfos, use: dnf debuginfo-install
glib2-2.64.6-1.fc32.x86_64 pcre-8.44-2.fc32.x86_64
(gdb) bt
#0 0x00007ffff7b839e5 in raise () from /lib64/libc.so.6
#1 0x00007ffff7b6c895 in abort () from /lib64/libc.so.6
#2 0x000055555557fe08 in error_handle_fatal (
errp=0x5555555aa300 <error_abort>, err=0x5555555b9580)
at ../util/error.c:40
#3 0x000055555557ff38 in error_setv (errp=0x5555555aa300 <error_abort>,
src=0x5555555914f1 "../util/qemu-config.c", line=24,
func=0x555555591ad0 <__func__.5> "find_list",
err_class=ERROR_CLASS_GENERIC_ERROR,
fmt=0x5555555914d3 "There is no option group '%s'", ap=0x7fffffffd690,
suffix=0x0) at ../util/error.c:73
#4 0x0000555555580116 in error_setg_internal (
errp=0x5555555aa300 <error_abort>,
src=0x5555555914f1 "../util/qemu-config.c", line=24,
func=0x555555591ad0 <__func__.5> "find_list",
fmt=0x5555555914d3 "There is no option group '%s'") at ../util/error.c:97
#5 0x000055555556fdb4 in find_list (lists=0x5555555aa060 <vm_config_groups>,
group=0x55555558e97a "object", errp=0x5555555aa300 <error_abort>)
at ../util/qemu-config.c:24
#6 0x0000555555570426 in qemu_find_opts_err (group=0x55555558e97a "object",
errp=0x5555555aa300 <error_abort>) at ../util/qemu-config.c:275
#7 0x000055555555f8bd in user_creatable_del (id=0x55555558e233 "dev0",
errp=0x5555555aa300 <error_abort>) at ../qom/object_interfaces.c:312
#8 0x000055555555dc8e in test_dummy_createcmdl ()
at ../tests/check-qom-proplist.c:439
#9 0x00007ffff7ef429e in g_test_run_suite_internal ()
from /lib64/libglib-2.0.so.0
#10 0x00007ffff7ef409b in g_test_run_suite_internal ()
from /lib64/libglib-2.0.so.0
#11 0x00007ffff7ef409b in g_test_run_suite_internal ()
from /lib64/libglib-2.0.so.0
#12 0x00007ffff7ef478a in g_test_run_suite () from /lib64/libglib-2.0.so.0
#13 0x00007ffff7ef47a5 in g_test_run () from /lib64/libglib-2.0.so.0
#14 0x000055555555e98c in main (argc=1, argv=0x7fffffffdcf8)
at ../tests/check-qom-proplist.c:655
> ---
> tests/check-qom-proplist.c | 58 +++++++++++++++++++++++++-------------
> 1 file changed, 38 insertions(+), 20 deletions(-)
>
> diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
> index 1b76581980..8dba26fb3c 100644
> --- a/tests/check-qom-proplist.c
> +++ b/tests/check-qom-proplist.c
> @@ -21,6 +21,8 @@
> #include "qemu/osdep.h"
>
> #include "qapi/error.h"
> +#include "qapi/qmp/qdict.h"
> +#include "qapi/qmp/qobject.h"
> #include "qom/object.h"
I expected this one to go.
> #include "qemu/module.h"
> #include "qemu/option.h"
> @@ -400,42 +402,58 @@ static void test_dummy_createlist(void)
>
> static void test_dummy_createcmdl(void)
> {
> - QemuOpts *opts;
> + QDict *qdict;
> DummyObject *dobj;
> Error *err = NULL;
> + bool help;
> const char *params = TYPE_DUMMY \
> ",id=dev0," \
> "bv=yes,sv=Hiss hiss hiss,av=platypus";
>
> - qemu_add_opts(&qemu_object_opts);
> - opts = qemu_opts_parse(&qemu_object_opts, params, true, &err);
> + qdict = keyval_parse(params, "qom-type", &help, &err);
Removes the only use of qemu_object_opts. You should remove
qemu_object_opts, too.
> g_assert(err == NULL);
> - g_assert(opts);
> + g_assert(qdict);
> + g_assert(!help);
>
> - dobj = DUMMY_OBJECT(user_creatable_add_opts(opts, &err));
> + g_assert(user_creatable_add_dict(qdict, true, &err));
> g_assert(err == NULL);
> + qobject_unref(qdict);
> +
> + dobj =
> DUMMY_OBJECT(object_resolve_path_component(object_get_objects_root(),
> + "dev0"));
Why does user_creatable_add_opts() return the object on success, null on
failure, but user_creatable_add_dict() only a rather less useful bool?
> 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);
Why parse again?
> + g_assert(!user_creatable_add_dict(qdict, true, &err));
> + g_assert(err);
Use error_free_or_abort(&err) instead, and ...
> + g_assert(object_resolve_path_component(object_get_objects_root(), "dev0")
> + == OBJECT(dobj));
> + qobject_unref(qdict);
... drop the next two lines:
> + error_free(err);
> + err = NULL;
> +
> + qdict = keyval_parse(params, "qom-type", &help, &err);
And again?
> 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(user_creatable_add_dict(qdict, true, &err));
Am I confused, or are you going from two to three creates? Should this
be in a separate patch?
> + 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)
