qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] test: QOM interface casting
@ 2013-12-20 13:26 Igor Mammedov
  2013-12-20 13:30 ` Peter Maydell
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Igor Mammedov @ 2013-12-20 13:26 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, afaerber, anthony, armbru

Add basic regression testing for QOM Interface usage.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
 - Peter Crosthwaite <peter.crosthwaite@xilinx.com>
   * s/Parent/parent_obj/
   * s/parent/parent_class/
 - Peter Maydell <peter.maydell@linaro.org>
   * s/interfacei/interface/
   * tests/Makefile split too long line
 - Andreas Färber <afaerber@suse.de>
   * consolidate QOM core object files in dedicated variable
   * add SoB and commit message
---
 tests/Makefile              |    6 ++-
 tests/check-qom-interface.c |  102 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 107 insertions(+), 1 deletions(-)
 create mode 100644 tests/check-qom-interface.c

diff --git a/tests/Makefile b/tests/Makefile
index 379cdd9..89f8eff 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -52,6 +52,8 @@ check-unit-y += tests/test-int128$(EXESUF)
 gcov-files-test-int128-y =
 check-unit-y += tests/test-bitops$(EXESUF)
 check-unit-y += tests/test-qdev-global-props$(EXESUF)
+check-unit-y = tests/check-qom-interface$(EXESUF)
+gcov-files-check-qdict-y = object/object.c
 
 check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
 
@@ -137,6 +139,7 @@ test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o
 
 $(test-obj-y): QEMU_INCLUDES += -Itests
 QEMU_CFLAGS += -I$(SRC_PATH)/tests
+qom-core-obj = qom/object.o qom/qom-qobject.o qom/container.o
 
 tests/test-x86-cpuid.o: QEMU_INCLUDES += -I$(SRC_PATH)/target-i386
 
@@ -146,6 +149,7 @@ tests/check-qdict$(EXESUF): tests/check-qdict.o libqemuutil.a
 tests/check-qlist$(EXESUF): tests/check-qlist.o libqemuutil.a
 tests/check-qfloat$(EXESUF): tests/check-qfloat.o libqemuutil.a
 tests/check-qjson$(EXESUF): tests/check-qjson.o libqemuutil.a libqemustub.a
+tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o $(qom-core-obj) libqemuutil.a
 tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(block-obj-y) libqemuutil.a libqemustub.a
 tests/test-aio$(EXESUF): tests/test-aio.o $(block-obj-y) libqemuutil.a libqemustub.a
 tests/test-throttle$(EXESUF): tests/test-throttle.o $(block-obj-y) libqemuutil.a libqemustub.a
@@ -159,7 +163,7 @@ tests/test-int128$(EXESUF): tests/test-int128.o
 tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
 	hw/core/qdev.o hw/core/qdev-properties.o \
 	hw/core/irq.o \
-	qom/object.o qom/container.o qom/qom-qobject.o \
+	$(qom-core-obj) \
 	$(test-qapi-obj-y) \
 	libqemuutil.a libqemustub.a
 
diff --git a/tests/check-qom-interface.c b/tests/check-qom-interface.c
new file mode 100644
index 0000000..8d7c1f8
--- /dev/null
+++ b/tests/check-qom-interface.c
@@ -0,0 +1,102 @@
+/*
+ * OQM interface test.
+ *
+ * Copyright (C) 2013 Red Hat Inc.
+ *
+ * Authors:
+ *  Igor Mammedov <imammedo@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ */
+#include <glib.h>
+
+#include "qom/object.h"
+#include "qemu/module.h"
+
+
+#define TYPE_TEST_IF "test-interface"
+#define TEST_IF_CLASS(klass) \
+     OBJECT_CLASS_CHECK(TestIfClass, (klass), TYPE_TEST_IF)
+#define TEST_IF_GET_CLASS(obj) \
+     OBJECT_GET_CLASS(TestIfClass, (obj), TYPE_TEST_IF)
+#define TEST_IF(obj) \
+     INTERFACE_CHECK(TestIf, (obj), TYPE_TEST_IF)
+
+typedef struct TestIf {
+    Object parent_obj;
+} TestIf;
+
+typedef struct TestIfClass {
+    InterfaceClass parent_class;
+
+    uint32_t test;
+} TestIfClass;
+
+static const TypeInfo test_if_info = {
+    .name          = TYPE_TEST_IF,
+    .parent        = TYPE_INTERFACE,
+    .class_size = sizeof(TestIfClass),
+};
+
+#define PATTERN 0xFAFBFCFD
+static void test_class_init(ObjectClass *oc, void *data)
+{
+    TestIfClass *tc = TEST_IF_CLASS(oc);
+
+    g_assert(tc);
+    tc->test = PATTERN;
+}
+
+#define TYPE_DIRECT_IMPL "direct-impl"
+static const TypeInfo direct_impl_info = {
+    .name = TYPE_DIRECT_IMPL,
+    .parent = TYPE_OBJECT,
+    .class_init = test_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_TEST_IF },
+        { }
+    }
+};
+
+#define TYPE_INTERMEDIATE_IMPL "intermediate-impl"
+static const TypeInfo intermediate_impl_info = {
+    .name = TYPE_INTERMEDIATE_IMPL,
+    .parent = TYPE_DIRECT_IMPL,
+};
+
+static void test_interface_impl(const char *type)
+{
+    Object *obj = object_new(type);
+    TestIf *iobj = TEST_IF(obj);
+    TestIfClass *ico = TEST_IF_GET_CLASS(iobj);
+
+    g_assert(iobj);
+    g_assert(ico->test == PATTERN);
+}
+
+static void interface_direct_test(void)
+{
+    test_interface_impl(TYPE_DIRECT_IMPL);
+}
+
+static void interface_intermediate_test(void)
+{
+    test_interface_impl(TYPE_INTERMEDIATE_IMPL);
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+
+    module_call_init(MODULE_INIT_QOM);
+    type_register_static(&test_if_info);
+    type_register_static(&direct_impl_info);
+    type_register_static(&intermediate_impl_info);
+
+    g_test_add_func("/interface/direct_impl", interface_direct_test);
+    g_test_add_func("/interface/intermediate_impl",
+                    interface_intermediate_test);
+
+    return g_test_run();
+}
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH] test: QOM interface casting
  2013-12-20 13:26 [Qemu-devel] [PATCH] test: QOM interface casting Igor Mammedov
@ 2013-12-20 13:30 ` Peter Maydell
  2013-12-20 13:58 ` Andreas Färber
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Peter Maydell @ 2013-12-20 13:30 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: Paolo Bonzini, Markus Armbruster, QEMU Developers,
	Anthony Liguori, Andreas Färber

On 20 December 2013 13:26, Igor Mammedov <imammedo@redhat.com> wrote:
> Add basic regression testing for QOM Interface usage.
>
> +/*
> + * OQM interface test.

Still says "OQM" not "QOM"...

thanks
-- PMM

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH] test: QOM interface casting
  2013-12-20 13:26 [Qemu-devel] [PATCH] test: QOM interface casting Igor Mammedov
  2013-12-20 13:30 ` Peter Maydell
@ 2013-12-20 13:58 ` Andreas Färber
  2013-12-20 14:08   ` Igor Mammedov
  2013-12-20 15:14 ` Igor Mammedov
  2013-12-20 15:54 ` Paolo Bonzini
  3 siblings, 1 reply; 8+ messages in thread
From: Andreas Färber @ 2013-12-20 13:58 UTC (permalink / raw)
  To: Igor Mammedov, qemu-devel
  Cc: Peter Maydell, Peter Crosthwaite, armbru, anthony, pbonzini

Am 20.12.2013 14:26, schrieb Igor Mammedov:
> Add basic regression testing for QOM Interface usage.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v2:
>  - Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>    * s/Parent/parent_obj/
>    * s/parent/parent_class/
>  - Peter Maydell <peter.maydell@linaro.org>
>    * s/interfacei/interface/
>    * tests/Makefile split too long line
>  - Andreas Färber <afaerber@suse.de>
>    * consolidate QOM core object files in dedicated variable
>    * add SoB and commit message
> ---
>  tests/Makefile              |    6 ++-
>  tests/check-qom-interface.c |  102 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 107 insertions(+), 1 deletions(-)
>  create mode 100644 tests/check-qom-interface.c
> 
> diff --git a/tests/Makefile b/tests/Makefile
> index 379cdd9..89f8eff 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -52,6 +52,8 @@ check-unit-y += tests/test-int128$(EXESUF)
>  gcov-files-test-int128-y =
>  check-unit-y += tests/test-bitops$(EXESUF)
>  check-unit-y += tests/test-qdev-global-props$(EXESUF)
> +check-unit-y = tests/check-qom-interface$(EXESUF)

Surely needs to be +=.

> +gcov-files-check-qdict-y = object/object.c

Copy&paste? ("qdict")
Also "qom/object.c".

>  
>  check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
>  
> @@ -137,6 +139,7 @@ test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o
>  
>  $(test-obj-y): QEMU_INCLUDES += -Itests
>  QEMU_CFLAGS += -I$(SRC_PATH)/tests
> +qom-core-obj = qom/object.o qom/qom-qobject.o qom/container.o
>  
>  tests/test-x86-cpuid.o: QEMU_INCLUDES += -I$(SRC_PATH)/target-i386
>  
> @@ -146,6 +149,7 @@ tests/check-qdict$(EXESUF): tests/check-qdict.o libqemuutil.a
>  tests/check-qlist$(EXESUF): tests/check-qlist.o libqemuutil.a
>  tests/check-qfloat$(EXESUF): tests/check-qfloat.o libqemuutil.a
>  tests/check-qjson$(EXESUF): tests/check-qjson.o libqemuutil.a libqemustub.a
> +tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o $(qom-core-obj) libqemuutil.a
>  tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(block-obj-y) libqemuutil.a libqemustub.a
>  tests/test-aio$(EXESUF): tests/test-aio.o $(block-obj-y) libqemuutil.a libqemustub.a
>  tests/test-throttle$(EXESUF): tests/test-throttle.o $(block-obj-y) libqemuutil.a libqemustub.a
> @@ -159,7 +163,7 @@ tests/test-int128$(EXESUF): tests/test-int128.o
>  tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
>  	hw/core/qdev.o hw/core/qdev-properties.o \
>  	hw/core/irq.o \
> -	qom/object.o qom/container.o qom/qom-qobject.o \
> +	$(qom-core-obj) \
>  	$(test-qapi-obj-y) \
>  	libqemuutil.a libqemustub.a
>  
> diff --git a/tests/check-qom-interface.c b/tests/check-qom-interface.c
> new file mode 100644
> index 0000000..8d7c1f8
> --- /dev/null
> +++ b/tests/check-qom-interface.c
> @@ -0,0 +1,102 @@
> +/*
> + * OQM interface test.
> + *
> + * Copyright (C) 2013 Red Hat Inc.
> + *
> + * Authors:
> + *  Igor Mammedov <imammedo@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + */
> +#include <glib.h>
> +
> +#include "qom/object.h"
> +#include "qemu/module.h"
> +
> +
> +#define TYPE_TEST_IF "test-interface"
> +#define TEST_IF_CLASS(klass) \
> +     OBJECT_CLASS_CHECK(TestIfClass, (klass), TYPE_TEST_IF)
> +#define TEST_IF_GET_CLASS(obj) \
> +     OBJECT_GET_CLASS(TestIfClass, (obj), TYPE_TEST_IF)
> +#define TEST_IF(obj) \
> +     INTERFACE_CHECK(TestIf, (obj), TYPE_TEST_IF)
> +
> +typedef struct TestIf {
> +    Object parent_obj;
> +} TestIf;
> +
> +typedef struct TestIfClass {
> +    InterfaceClass parent_class;
> +
> +    uint32_t test;
> +} TestIfClass;
> +
> +static const TypeInfo test_if_info = {
> +    .name          = TYPE_TEST_IF,
> +    .parent        = TYPE_INTERFACE,

TestIf is not assigned here - no bug but let's set a consistent example.

> +    .class_size = sizeof(TestIfClass),
> +};
> +
> +#define PATTERN 0xFAFBFCFD
> +static void test_class_init(ObjectClass *oc, void *data)
> +{
> +    TestIfClass *tc = TEST_IF_CLASS(oc);
> +
> +    g_assert(tc);
> +    tc->test = PATTERN;
> +}
> +
> +#define TYPE_DIRECT_IMPL "direct-impl"
> +static const TypeInfo direct_impl_info = {
> +    .name = TYPE_DIRECT_IMPL,
> +    .parent = TYPE_OBJECT,
> +    .class_init = test_class_init,
> +    .interfaces = (InterfaceInfo[]) {
> +        { TYPE_TEST_IF },
> +        { }
> +    }
> +};
> +
> +#define TYPE_INTERMEDIATE_IMPL "intermediate-impl"

I'd insert a white line here and above.

> +static const TypeInfo intermediate_impl_info = {
> +    .name = TYPE_INTERMEDIATE_IMPL,
> +    .parent = TYPE_DIRECT_IMPL,
> +};
> +
> +static void test_interface_impl(const char *type)
> +{
> +    Object *obj = object_new(type);
> +    TestIf *iobj = TEST_IF(obj);
> +    TestIfClass *ico = TEST_IF_GET_CLASS(iobj);

What's the o in ico for BTW?

> +
> +    g_assert(iobj);
> +    g_assert(ico->test == PATTERN);
> +}
> +
> +static void interface_direct_test(void)
> +{
> +    test_interface_impl(TYPE_DIRECT_IMPL);
> +}
> +
> +static void interface_intermediate_test(void)
> +{
> +    test_interface_impl(TYPE_INTERMEDIATE_IMPL);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    g_test_init(&argc, &argv, NULL);
> +
> +    module_call_init(MODULE_INIT_QOM);
> +    type_register_static(&test_if_info);
> +    type_register_static(&direct_impl_info);
> +    type_register_static(&intermediate_impl_info);
> +
> +    g_test_add_func("/interface/direct_impl", interface_direct_test);
> +    g_test_add_func("/interface/intermediate_impl",

Would it make sense to call it /qom/interface/... or /qom-interface/...?
Doesn't really matter though as long as it's unique, I guess.

> +                    interface_intermediate_test);
> +
> +    return g_test_run();
> +}

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH] test: QOM interface casting
  2013-12-20 13:58 ` Andreas Färber
@ 2013-12-20 14:08   ` Igor Mammedov
  2013-12-20 21:27     ` Peter Crosthwaite
  0 siblings, 1 reply; 8+ messages in thread
From: Igor Mammedov @ 2013-12-20 14:08 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Peter Maydell, Peter Crosthwaite, qemu-devel, armbru, anthony,
	pbonzini

On Fri, 20 Dec 2013 14:58:07 +0100
Andreas Färber <afaerber@suse.de> wrote:

> Am 20.12.2013 14:26, schrieb Igor Mammedov:
> > Add basic regression testing for QOM Interface usage.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > v2:
> >  - Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> >    * s/Parent/parent_obj/
> >    * s/parent/parent_class/
> >  - Peter Maydell <peter.maydell@linaro.org>
> >    * s/interfacei/interface/
> >    * tests/Makefile split too long line
> >  - Andreas Färber <afaerber@suse.de>
> >    * consolidate QOM core object files in dedicated variable
> >    * add SoB and commit message
> > ---
> >  tests/Makefile              |    6 ++-
> >  tests/check-qom-interface.c |  102 +++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 107 insertions(+), 1 deletions(-)
> >  create mode 100644 tests/check-qom-interface.c
> > 
> > diff --git a/tests/Makefile b/tests/Makefile
> > index 379cdd9..89f8eff 100644
> > --- a/tests/Makefile
> > +++ b/tests/Makefile
> > @@ -52,6 +52,8 @@ check-unit-y += tests/test-int128$(EXESUF)
> >  gcov-files-test-int128-y =
> >  check-unit-y += tests/test-bitops$(EXESUF)
> >  check-unit-y += tests/test-qdev-global-props$(EXESUF)
> > +check-unit-y = tests/check-qom-interface$(EXESUF)
> 
> Surely needs to be +=.
fixed.
> 
> > +gcov-files-check-qdict-y = object/object.c
> 
> Copy&paste? ("qdict")
> Also "qom/object.c".
fixed

> 
> >  
> >  check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
> >  
> > @@ -137,6 +139,7 @@ test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o
> >  
> >  $(test-obj-y): QEMU_INCLUDES += -Itests
> >  QEMU_CFLAGS += -I$(SRC_PATH)/tests
> > +qom-core-obj = qom/object.o qom/qom-qobject.o qom/container.o
> >  
> >  tests/test-x86-cpuid.o: QEMU_INCLUDES += -I$(SRC_PATH)/target-i386
> >  
> > @@ -146,6 +149,7 @@ tests/check-qdict$(EXESUF): tests/check-qdict.o libqemuutil.a
> >  tests/check-qlist$(EXESUF): tests/check-qlist.o libqemuutil.a
> >  tests/check-qfloat$(EXESUF): tests/check-qfloat.o libqemuutil.a
> >  tests/check-qjson$(EXESUF): tests/check-qjson.o libqemuutil.a libqemustub.a
> > +tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o $(qom-core-obj) libqemuutil.a
> >  tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(block-obj-y) libqemuutil.a libqemustub.a
> >  tests/test-aio$(EXESUF): tests/test-aio.o $(block-obj-y) libqemuutil.a libqemustub.a
> >  tests/test-throttle$(EXESUF): tests/test-throttle.o $(block-obj-y) libqemuutil.a libqemustub.a
> > @@ -159,7 +163,7 @@ tests/test-int128$(EXESUF): tests/test-int128.o
> >  tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
> >  	hw/core/qdev.o hw/core/qdev-properties.o \
> >  	hw/core/irq.o \
> > -	qom/object.o qom/container.o qom/qom-qobject.o \
> > +	$(qom-core-obj) \
> >  	$(test-qapi-obj-y) \
> >  	libqemuutil.a libqemustub.a
> >  
> > diff --git a/tests/check-qom-interface.c b/tests/check-qom-interface.c
> > new file mode 100644
> > index 0000000..8d7c1f8
> > --- /dev/null
> > +++ b/tests/check-qom-interface.c
> > @@ -0,0 +1,102 @@
> > +/*
> > + * OQM interface test.
> > + *
> > + * Copyright (C) 2013 Red Hat Inc.
> > + *
> > + * Authors:
> > + *  Igor Mammedov <imammedo@redhat.com>
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> > + * See the COPYING.LIB file in the top-level directory.
> > + */
> > +#include <glib.h>
> > +
> > +#include "qom/object.h"
> > +#include "qemu/module.h"
> > +
> > +
> > +#define TYPE_TEST_IF "test-interface"
> > +#define TEST_IF_CLASS(klass) \
> > +     OBJECT_CLASS_CHECK(TestIfClass, (klass), TYPE_TEST_IF)
> > +#define TEST_IF_GET_CLASS(obj) \
> > +     OBJECT_GET_CLASS(TestIfClass, (obj), TYPE_TEST_IF)
> > +#define TEST_IF(obj) \
> > +     INTERFACE_CHECK(TestIf, (obj), TYPE_TEST_IF)
> > +
> > +typedef struct TestIf {
> > +    Object parent_obj;
> > +} TestIf;
> > +
> > +typedef struct TestIfClass {
> > +    InterfaceClass parent_class;
> > +
> > +    uint32_t test;
> > +} TestIfClass;
> > +
> > +static const TypeInfo test_if_info = {
> > +    .name          = TYPE_TEST_IF,
> > +    .parent        = TYPE_INTERFACE,
> 
> TestIf is not assigned here - no bug but let's set a consistent example.
What do you mean?

> 
> > +    .class_size = sizeof(TestIfClass),
> > +};
> > +
> > +#define PATTERN 0xFAFBFCFD
> > +static void test_class_init(ObjectClass *oc, void *data)
> > +{
> > +    TestIfClass *tc = TEST_IF_CLASS(oc);
> > +
> > +    g_assert(tc);
> > +    tc->test = PATTERN;
> > +}
> > +
> > +#define TYPE_DIRECT_IMPL "direct-impl"
> > +static const TypeInfo direct_impl_info = {
> > +    .name = TYPE_DIRECT_IMPL,
> > +    .parent = TYPE_OBJECT,
> > +    .class_init = test_class_init,
> > +    .interfaces = (InterfaceInfo[]) {
> > +        { TYPE_TEST_IF },
> > +        { }
> > +    }
> > +};
> > +
> > +#define TYPE_INTERMEDIATE_IMPL "intermediate-impl"
> 
> I'd insert a white line here and above.
sure

> 
> > +static const TypeInfo intermediate_impl_info = {
> > +    .name = TYPE_INTERMEDIATE_IMPL,
> > +    .parent = TYPE_DIRECT_IMPL,
> > +};
> > +
> > +static void test_interface_impl(const char *type)
> > +{
> > +    Object *obj = object_new(type);
> > +    TestIf *iobj = TEST_IF(obj);
> > +    TestIfClass *ico = TEST_IF_GET_CLASS(iobj);
> 
> What's the o in ico for BTW?
InterfaceClassObject ...

> 
> > +
> > +    g_assert(iobj);
> > +    g_assert(ico->test == PATTERN);
> > +}
> > +
> > +static void interface_direct_test(void)
> > +{
> > +    test_interface_impl(TYPE_DIRECT_IMPL);
> > +}
> > +
> > +static void interface_intermediate_test(void)
> > +{
> > +    test_interface_impl(TYPE_INTERMEDIATE_IMPL);
> > +}
> > +
> > +int main(int argc, char **argv)
> > +{
> > +    g_test_init(&argc, &argv, NULL);
> > +
> > +    module_call_init(MODULE_INIT_QOM);
> > +    type_register_static(&test_if_info);
> > +    type_register_static(&direct_impl_info);
> > +    type_register_static(&intermediate_impl_info);
> > +
> > +    g_test_add_func("/interface/direct_impl", interface_direct_test);
> > +    g_test_add_func("/interface/intermediate_impl",
> 
> Would it make sense to call it /qom/interface/... or /qom-interface/...?
sure, I'll amend it.

> Doesn't really matter though as long as it's unique, I guess.
> 
> > +                    interface_intermediate_test);
> > +
> > +    return g_test_run();
> > +}
> 
> Regards,
> Andreas
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH] test: QOM interface casting
  2013-12-20 13:26 [Qemu-devel] [PATCH] test: QOM interface casting Igor Mammedov
  2013-12-20 13:30 ` Peter Maydell
  2013-12-20 13:58 ` Andreas Färber
@ 2013-12-20 15:14 ` Igor Mammedov
  2013-12-20 15:56   ` Paolo Bonzini
  2013-12-20 15:54 ` Paolo Bonzini
  3 siblings, 1 reply; 8+ messages in thread
From: Igor Mammedov @ 2013-12-20 15:14 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: pbonzini, armbru, qemu-devel, anthony, afaerber

On Fri, 20 Dec 2013 14:26:11 +0100
Igor Mammedov <imammedo@redhat.com> wrote:

Andreas,

test shows that commit:

"qom: Do not register interface "types" in the type table "
https://github.com/afaerber/qemu-cpu/commit/7f00136ff5534ee651f4f10475170b8db18e5c03

regresses QOM Interface in case where interface is implemented
in not a leaf class, leading to abort.

> Add basic regression testing for QOM Interface usage.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v2:
>  - Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>    * s/Parent/parent_obj/
>    * s/parent/parent_class/
>  - Peter Maydell <peter.maydell@linaro.org>
>    * s/interfacei/interface/
>    * tests/Makefile split too long line
>  - Andreas Färber <afaerber@suse.de>
>    * consolidate QOM core object files in dedicated variable
>    * add SoB and commit message
> ---
>  tests/Makefile              |    6 ++-
>  tests/check-qom-interface.c |  102 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 107 insertions(+), 1 deletions(-)
>  create mode 100644 tests/check-qom-interface.c
> 
> diff --git a/tests/Makefile b/tests/Makefile
> index 379cdd9..89f8eff 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -52,6 +52,8 @@ check-unit-y += tests/test-int128$(EXESUF)
>  gcov-files-test-int128-y =
>  check-unit-y += tests/test-bitops$(EXESUF)
>  check-unit-y += tests/test-qdev-global-props$(EXESUF)
> +check-unit-y = tests/check-qom-interface$(EXESUF)
> +gcov-files-check-qdict-y = object/object.c
>  
>  check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
>  
> @@ -137,6 +139,7 @@ test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o
>  
>  $(test-obj-y): QEMU_INCLUDES += -Itests
>  QEMU_CFLAGS += -I$(SRC_PATH)/tests
> +qom-core-obj = qom/object.o qom/qom-qobject.o qom/container.o
>  
>  tests/test-x86-cpuid.o: QEMU_INCLUDES += -I$(SRC_PATH)/target-i386
>  
> @@ -146,6 +149,7 @@ tests/check-qdict$(EXESUF): tests/check-qdict.o libqemuutil.a
>  tests/check-qlist$(EXESUF): tests/check-qlist.o libqemuutil.a
>  tests/check-qfloat$(EXESUF): tests/check-qfloat.o libqemuutil.a
>  tests/check-qjson$(EXESUF): tests/check-qjson.o libqemuutil.a libqemustub.a
> +tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o $(qom-core-obj) libqemuutil.a
>  tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(block-obj-y) libqemuutil.a libqemustub.a
>  tests/test-aio$(EXESUF): tests/test-aio.o $(block-obj-y) libqemuutil.a libqemustub.a
>  tests/test-throttle$(EXESUF): tests/test-throttle.o $(block-obj-y) libqemuutil.a libqemustub.a
> @@ -159,7 +163,7 @@ tests/test-int128$(EXESUF): tests/test-int128.o
>  tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
>  	hw/core/qdev.o hw/core/qdev-properties.o \
>  	hw/core/irq.o \
> -	qom/object.o qom/container.o qom/qom-qobject.o \
> +	$(qom-core-obj) \
>  	$(test-qapi-obj-y) \
>  	libqemuutil.a libqemustub.a
>  
> diff --git a/tests/check-qom-interface.c b/tests/check-qom-interface.c
> new file mode 100644
> index 0000000..8d7c1f8
> --- /dev/null
> +++ b/tests/check-qom-interface.c
> @@ -0,0 +1,102 @@
> +/*
> + * OQM interface test.
> + *
> + * Copyright (C) 2013 Red Hat Inc.
> + *
> + * Authors:
> + *  Igor Mammedov <imammedo@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + */
> +#include <glib.h>
> +
> +#include "qom/object.h"
> +#include "qemu/module.h"
> +
> +
> +#define TYPE_TEST_IF "test-interface"
> +#define TEST_IF_CLASS(klass) \
> +     OBJECT_CLASS_CHECK(TestIfClass, (klass), TYPE_TEST_IF)
> +#define TEST_IF_GET_CLASS(obj) \
> +     OBJECT_GET_CLASS(TestIfClass, (obj), TYPE_TEST_IF)
> +#define TEST_IF(obj) \
> +     INTERFACE_CHECK(TestIf, (obj), TYPE_TEST_IF)
> +
> +typedef struct TestIf {
> +    Object parent_obj;
> +} TestIf;
> +
> +typedef struct TestIfClass {
> +    InterfaceClass parent_class;
> +
> +    uint32_t test;
> +} TestIfClass;
> +
> +static const TypeInfo test_if_info = {
> +    .name          = TYPE_TEST_IF,
> +    .parent        = TYPE_INTERFACE,
> +    .class_size = sizeof(TestIfClass),
> +};
> +
> +#define PATTERN 0xFAFBFCFD
> +static void test_class_init(ObjectClass *oc, void *data)
> +{
> +    TestIfClass *tc = TEST_IF_CLASS(oc);
> +
> +    g_assert(tc);
> +    tc->test = PATTERN;
> +}
> +
> +#define TYPE_DIRECT_IMPL "direct-impl"
> +static const TypeInfo direct_impl_info = {
> +    .name = TYPE_DIRECT_IMPL,
> +    .parent = TYPE_OBJECT,
> +    .class_init = test_class_init,
> +    .interfaces = (InterfaceInfo[]) {
> +        { TYPE_TEST_IF },
> +        { }
> +    }
> +};
> +
> +#define TYPE_INTERMEDIATE_IMPL "intermediate-impl"
> +static const TypeInfo intermediate_impl_info = {
> +    .name = TYPE_INTERMEDIATE_IMPL,
> +    .parent = TYPE_DIRECT_IMPL,
> +};
> +
> +static void test_interface_impl(const char *type)
> +{
> +    Object *obj = object_new(type);
> +    TestIf *iobj = TEST_IF(obj);
> +    TestIfClass *ico = TEST_IF_GET_CLASS(iobj);
> +
> +    g_assert(iobj);
> +    g_assert(ico->test == PATTERN);
> +}
> +
> +static void interface_direct_test(void)
> +{
> +    test_interface_impl(TYPE_DIRECT_IMPL);
> +}
> +
> +static void interface_intermediate_test(void)
> +{
> +    test_interface_impl(TYPE_INTERMEDIATE_IMPL);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    g_test_init(&argc, &argv, NULL);
> +
> +    module_call_init(MODULE_INIT_QOM);
> +    type_register_static(&test_if_info);
> +    type_register_static(&direct_impl_info);
> +    type_register_static(&intermediate_impl_info);
> +
> +    g_test_add_func("/interface/direct_impl", interface_direct_test);
> +    g_test_add_func("/interface/intermediate_impl",
> +                    interface_intermediate_test);
> +
> +    return g_test_run();
> +}

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH] test: QOM interface casting
  2013-12-20 13:26 [Qemu-devel] [PATCH] test: QOM interface casting Igor Mammedov
                   ` (2 preceding siblings ...)
  2013-12-20 15:14 ` Igor Mammedov
@ 2013-12-20 15:54 ` Paolo Bonzini
  3 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2013-12-20 15:54 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: armbru, qemu-devel, anthony, afaerber

Il 20/12/2013 14:26, Igor Mammedov ha scritto:
> index 379cdd9..89f8eff 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -52,6 +52,8 @@ check-unit-y += tests/test-int128$(EXESUF)
>  gcov-files-test-int128-y =
>  check-unit-y += tests/test-bitops$(EXESUF)
>  check-unit-y += tests/test-qdev-global-props$(EXESUF)
> +check-unit-y = tests/check-qom-interface$(EXESUF)
> +gcov-files-check-qdict-y = object/object.c

The "=" needs to be a "+=" and the gcov-files-check-qdict-y variable is
wrong (both in name and in content; it should be qom/object.c).

The test itself is good and, as you mentioned on IRC, spots a bug.

Paolo

>  
>  check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH] test: QOM interface casting
  2013-12-20 15:14 ` Igor Mammedov
@ 2013-12-20 15:56   ` Paolo Bonzini
  0 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2013-12-20 15:56 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: afaerber, armbru, anthony, qemu-devel

Il 20/12/2013 16:14, Igor Mammedov ha scritto:
> Andreas,
> 
> test shows that commit:
> 
> "qom: Do not register interface "types" in the type table "
> https://github.com/afaerber/qemu-cpu/commit/7f00136ff5534ee651f4f10475170b8db18e5c03
> 
> regresses QOM Interface in case where interface is implemented
> in not a leaf class, leading to abort.

Just sent a patch for this.

Paolo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH] test: QOM interface casting
  2013-12-20 14:08   ` Igor Mammedov
@ 2013-12-20 21:27     ` Peter Crosthwaite
  0 siblings, 0 replies; 8+ messages in thread
From: Peter Crosthwaite @ 2013-12-20 21:27 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: Peter Maydell, Markus Armbruster,
	qemu-devel@nongnu.org Developers, Anthony Liguori, Paolo Bonzini,
	Andreas Färber

On Sat, Dec 21, 2013 at 12:08 AM, Igor Mammedov <imammedo@redhat.com> wrote:
> On Fri, 20 Dec 2013 14:58:07 +0100
> Andreas Färber <afaerber@suse.de> wrote:
>
>> Am 20.12.2013 14:26, schrieb Igor Mammedov:
>> > Add basic regression testing for QOM Interface usage.
>> >
>> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>> > ---
>> > v2:
>> >  - Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> >    * s/Parent/parent_obj/
>> >    * s/parent/parent_class/
>> >  - Peter Maydell <peter.maydell@linaro.org>
>> >    * s/interfacei/interface/
>> >    * tests/Makefile split too long line
>> >  - Andreas Färber <afaerber@suse.de>
>> >    * consolidate QOM core object files in dedicated variable
>> >    * add SoB and commit message
>> > ---
>> >  tests/Makefile              |    6 ++-
>> >  tests/check-qom-interface.c |  102 +++++++++++++++++++++++++++++++++++++++++++
>> >  2 files changed, 107 insertions(+), 1 deletions(-)
>> >  create mode 100644 tests/check-qom-interface.c
>> >
>> > diff --git a/tests/Makefile b/tests/Makefile
>> > index 379cdd9..89f8eff 100644
>> > --- a/tests/Makefile
>> > +++ b/tests/Makefile
>> > @@ -52,6 +52,8 @@ check-unit-y += tests/test-int128$(EXESUF)
>> >  gcov-files-test-int128-y =
>> >  check-unit-y += tests/test-bitops$(EXESUF)
>> >  check-unit-y += tests/test-qdev-global-props$(EXESUF)
>> > +check-unit-y = tests/check-qom-interface$(EXESUF)
>>
>> Surely needs to be +=.
> fixed.
>>
>> > +gcov-files-check-qdict-y = object/object.c
>>
>> Copy&paste? ("qdict")
>> Also "qom/object.c".
> fixed
>
>>
>> >
>> >  check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
>> >
>> > @@ -137,6 +139,7 @@ test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o
>> >
>> >  $(test-obj-y): QEMU_INCLUDES += -Itests
>> >  QEMU_CFLAGS += -I$(SRC_PATH)/tests
>> > +qom-core-obj = qom/object.o qom/qom-qobject.o qom/container.o
>> >
>> >  tests/test-x86-cpuid.o: QEMU_INCLUDES += -I$(SRC_PATH)/target-i386
>> >
>> > @@ -146,6 +149,7 @@ tests/check-qdict$(EXESUF): tests/check-qdict.o libqemuutil.a
>> >  tests/check-qlist$(EXESUF): tests/check-qlist.o libqemuutil.a
>> >  tests/check-qfloat$(EXESUF): tests/check-qfloat.o libqemuutil.a
>> >  tests/check-qjson$(EXESUF): tests/check-qjson.o libqemuutil.a libqemustub.a
>> > +tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o $(qom-core-obj) libqemuutil.a
>> >  tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(block-obj-y) libqemuutil.a libqemustub.a
>> >  tests/test-aio$(EXESUF): tests/test-aio.o $(block-obj-y) libqemuutil.a libqemustub.a
>> >  tests/test-throttle$(EXESUF): tests/test-throttle.o $(block-obj-y) libqemuutil.a libqemustub.a
>> > @@ -159,7 +163,7 @@ tests/test-int128$(EXESUF): tests/test-int128.o
>> >  tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
>> >     hw/core/qdev.o hw/core/qdev-properties.o \
>> >     hw/core/irq.o \
>> > -   qom/object.o qom/container.o qom/qom-qobject.o \
>> > +   $(qom-core-obj) \
>> >     $(test-qapi-obj-y) \
>> >     libqemuutil.a libqemustub.a
>> >
>> > diff --git a/tests/check-qom-interface.c b/tests/check-qom-interface.c
>> > new file mode 100644
>> > index 0000000..8d7c1f8
>> > --- /dev/null
>> > +++ b/tests/check-qom-interface.c
>> > @@ -0,0 +1,102 @@
>> > +/*
>> > + * OQM interface test.
>> > + *
>> > + * Copyright (C) 2013 Red Hat Inc.
>> > + *
>> > + * Authors:
>> > + *  Igor Mammedov <imammedo@redhat.com>
>> > + *
>> > + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
>> > + * See the COPYING.LIB file in the top-level directory.
>> > + */
>> > +#include <glib.h>
>> > +
>> > +#include "qom/object.h"
>> > +#include "qemu/module.h"
>> > +
>> > +
>> > +#define TYPE_TEST_IF "test-interface"
>> > +#define TEST_IF_CLASS(klass) \
>> > +     OBJECT_CLASS_CHECK(TestIfClass, (klass), TYPE_TEST_IF)
>> > +#define TEST_IF_GET_CLASS(obj) \
>> > +     OBJECT_GET_CLASS(TestIfClass, (obj), TYPE_TEST_IF)
>> > +#define TEST_IF(obj) \
>> > +     INTERFACE_CHECK(TestIf, (obj), TYPE_TEST_IF)
>> > +
>> > +typedef struct TestIf {
>> > +    Object parent_obj;
>> > +} TestIf;
>> > +
>> > +typedef struct TestIfClass {
>> > +    InterfaceClass parent_class;
>> > +
>> > +    uint32_t test;
>> > +} TestIfClass;
>> > +
>> > +static const TypeInfo test_if_info = {
>> > +    .name          = TYPE_TEST_IF,
>> > +    .parent        = TYPE_INTERFACE,
>>
>> TestIf is not assigned here - no bug but let's set a consistent example.
> What do you mean?
>
>>
>> > +    .class_size = sizeof(TestIfClass),
>> > +};
>> > +
>> > +#define PATTERN 0xFAFBFCFD
>> > +static void test_class_init(ObjectClass *oc, void *data)
>> > +{
>> > +    TestIfClass *tc = TEST_IF_CLASS(oc);
>> > +
>> > +    g_assert(tc);
>> > +    tc->test = PATTERN;
>> > +}
>> > +
>> > +#define TYPE_DIRECT_IMPL "direct-impl"
>> > +static const TypeInfo direct_impl_info = {
>> > +    .name = TYPE_DIRECT_IMPL,
>> > +    .parent = TYPE_OBJECT,
>> > +    .class_init = test_class_init,
>> > +    .interfaces = (InterfaceInfo[]) {
>> > +        { TYPE_TEST_IF },
>> > +        { }
>> > +    }
>> > +};
>> > +
>> > +#define TYPE_INTERMEDIATE_IMPL "intermediate-impl"
>>
>> I'd insert a white line here and above.
> sure
>
>>
>> > +static const TypeInfo intermediate_impl_info = {
>> > +    .name = TYPE_INTERMEDIATE_IMPL,
>> > +    .parent = TYPE_DIRECT_IMPL,
>> > +};
>> > +
>> > +static void test_interface_impl(const char *type)
>> > +{
>> > +    Object *obj = object_new(type);
>> > +    TestIf *iobj = TEST_IF(obj);
>> > +    TestIfClass *ico = TEST_IF_GET_CLASS(iobj);
>>
>> What's the o in ico for BTW?
> InterfaceClassObject ...
>

This seems strange to me. In some other code I rmbr just using lower
case acronym versions of the type to keep it all consistent. This
means you could use "tic" for TestIfClass.

Regards,
Peter

>>
>> > +
>> > +    g_assert(iobj);
>> > +    g_assert(ico->test == PATTERN);
>> > +}
>> > +
>> > +static void interface_direct_test(void)
>> > +{
>> > +    test_interface_impl(TYPE_DIRECT_IMPL);
>> > +}
>> > +
>> > +static void interface_intermediate_test(void)
>> > +{
>> > +    test_interface_impl(TYPE_INTERMEDIATE_IMPL);
>> > +}
>> > +
>> > +int main(int argc, char **argv)
>> > +{
>> > +    g_test_init(&argc, &argv, NULL);
>> > +
>> > +    module_call_init(MODULE_INIT_QOM);
>> > +    type_register_static(&test_if_info);
>> > +    type_register_static(&direct_impl_info);
>> > +    type_register_static(&intermediate_impl_info);
>> > +
>> > +    g_test_add_func("/interface/direct_impl", interface_direct_test);
>> > +    g_test_add_func("/interface/intermediate_impl",
>>
>> Would it make sense to call it /qom/interface/... or /qom-interface/...?
> sure, I'll amend it.
>
>> Doesn't really matter though as long as it's unique, I guess.
>>
>> > +                    interface_intermediate_test);
>> > +
>> > +    return g_test_run();
>> > +}
>>
>> Regards,
>> Andreas
>>
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-12-20 21:27 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-20 13:26 [Qemu-devel] [PATCH] test: QOM interface casting Igor Mammedov
2013-12-20 13:30 ` Peter Maydell
2013-12-20 13:58 ` Andreas Färber
2013-12-20 14:08   ` Igor Mammedov
2013-12-20 21:27     ` Peter Crosthwaite
2013-12-20 15:14 ` Igor Mammedov
2013-12-20 15:56   ` Paolo Bonzini
2013-12-20 15:54 ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).