[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.8 2/2] tests: check-qom-proplist: add chec
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH for-2.8 2/2] tests: check-qom-proplist: add checks for cmdline-created objects |
Date: |
Wed, 07 Dec 2016 12:10:47 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
"Daniel P. Berrange" <address@hidden> writes:
> On Tue, Dec 06, 2016 at 02:15:00PM -0600, Michael Roth wrote:
>> check-qom-proplist originally added tests for verifying that
>> object-creation helpers object_new_with_{props,propv} behaved in
>> similar fashion to the "traditional" method involving setting each
>> individual property separately after object creation rather than
>> in via a single call.
>>
>> Another similar "helper" for creating Objects exists in the form of
>> objects specified via -object command-line parameters. By that
>> rationale, we extend check-qom-proplist to include similar checks
>> for command-line-created objects by employing the same
>> qemu_opts_parse()-based parsing the vl.c employs.
>>
>> This parser has a side-effect of parsing the object's options into
>> a QemuOpt structure and registering this in the global QemuOptsList
>> using the Object's ID. This can conflict with future Object instances
>> that attempt to use the same ID if we don't ensure this is cleaned
>> up as part of Object finalization, so we add specific checks for this
>> scenario in addition to the normal sanity checks.
>>
>> Suggested-by: Daniel Berrange <address@hidden>
>> Cc: "Dr. David Alan Gilbert" <address@hidden>
>> Cc: Markus Armbruster <address@hidden>
>> Cc: Eric Blake <address@hidden>
>> Cc: Daniel Berrange <address@hidden>
>> Signed-off-by: Michael Roth <address@hidden>
>> ---
>> tests/check-qom-proplist.c | 54
>> +++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 53 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
>> index a16cefc..087ce21 100644
>> --- a/tests/check-qom-proplist.c
>> +++ b/tests/check-qom-proplist.c
>> @@ -23,6 +23,9 @@
>> #include "qapi/error.h"
>> #include "qom/object.h"
>> #include "qemu/module.h"
>> +#include "qemu/option.h"
>> +#include "qemu/config-file.h"
>> +#include "qom/object_interfaces.h"
>>
>>
>> #define TYPE_DUMMY "qemu-dummy"
>> @@ -162,6 +165,10 @@ static const TypeInfo dummy_info = {
>> .instance_finalize = dummy_finalize,
>> .class_size = sizeof(DummyObjectClass),
>> .class_init = dummy_class_init,
>> + .interfaces = (InterfaceInfo[]) {
>> + { TYPE_USER_CREATABLE },
>> + { }
>> + }
>> };
>>
>>
>> @@ -291,7 +298,6 @@ static void dummy_backend_init(Object *obj)
>> {
>> }
>>
>> -
>> static const TypeInfo dummy_dev_info = {
>> .name = TYPE_DUMMY_DEV,
>> .parent = TYPE_OBJECT,
>> @@ -320,6 +326,14 @@ static const TypeInfo dummy_backend_info = {
>> .class_size = sizeof(DummyBackendClass),
>> };
>>
>> +static QemuOptsList qemu_object_opts = {
>> + .name = "object",
>> + .implied_opt_name = "qom-type",
>> + .head = QTAILQ_HEAD_INITIALIZER(qemu_object_opts.head),
>> + .desc = {
>> + { }
>> + },
>> +};
>>
>>
>> static void test_dummy_createv(void)
>> @@ -388,6 +402,43 @@ static void test_dummy_createlist(void)
>> object_unparent(OBJECT(dobj));
>> }
>>
>> +static void test_dummy_createcmdl(void)
>> +{
>> + QemuOpts *opts;
>> + DummyObject *dobj;
>> + Error *err = NULL;
>> + 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);
>> + g_assert(err == NULL);
>> + g_assert(opts);
>> +
>> + dobj = DUMMY_OBJECT(user_creatable_add_opts(opts, &err));
>> + g_assert(err == NULL);
>> + g_assert(dobj);
>> + g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss");
>> + g_assert(dobj->bv == true);
>> + g_assert(dobj->av == DUMMY_PLATYPUS);
>> +
>> + user_creatable_del("dev0", &err);
>> + g_assert(err == NULL);
>> + error_free(err);
>> +
>> + /* 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 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(qemu_opts_find(&qemu_object_opts, "dev0") == NULL);
>> +}
>> +
>> static void test_dummy_badenum(void)
>> {
>> Error *err = NULL;
>> @@ -525,6 +576,7 @@ int main(int argc, char **argv)
>>
>> g_test_add_func("/qom/proplist/createlist", test_dummy_createlist);
>> g_test_add_func("/qom/proplist/createv", test_dummy_createv);
>> + g_test_add_func("/qom/proplist/createcmdline", test_dummy_createcmdl);
>> g_test_add_func("/qom/proplist/badenum", test_dummy_badenum);
>> g_test_add_func("/qom/proplist/getenum", test_dummy_getenum);
>> g_test_add_func("/qom/proplist/iterator", test_dummy_iterator);
>
> Reviewed-by: Daniel P. Berrange <address@hidden>
>
>
> except that this should be squashed into the previous commit - tests should
> always be in the same commit as the new code that they're verifying.
Not sure how particular we are about that.
When neither test nor fix is trivial, I personally like to commit the
test first, defanged so it doesn't blow up make check, with a suitable
FIXME comment. The fix then resolves the FIXME.