Re: [PATCH 06/25] tests: convert check-qom-proplist to keyval

[Paolo Bonzini]

On 22/01/21 15:14, Markus Armbruster wrote:
> ok 2 /qom/proplist/createv
> Unexpected error in find_list() at ../util/qemu-config.c:24:
> There is no option group 'object'

Hmm, maybe a semantic conflict when I rebased.  I'll take a look.

> > +    qdict = keyval_parse(params, "qom-type", &help, &err);
>
> Why parse again?

I don't remember why I did it that way, but it does seem cleaner. 
Unlike QemuOpts, which are persistent until qemu_opts_del and can be 
retrieved later, keyval's result only survives as long as you want it to 
survive, and are leaked if you don't unref the qdict.  Parsing every 
time the object is created seems more similar to the way the vl.c code 
works.

> > -     * 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?

It's a different way to test for "you can create another object with the 
same id" you had before.  It used to check for NULL qemu_opts_find, now 
it checks directly that the creation succeeds.  I can put it in the 
commit message.

Paolo

> > +    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));