Message ID | 1481738323-9147-1-git-send-email-ehabkost@redhat.com |
---|---|
State | New |
Headers | show |
Eduardo Habkost <ehabkost@redhat.com> writes: > "qom-list-types abstract=false" currently returns all interface > types, as if they were not abstract. Fix this by making sure all > interface types are abstract. > > All interface types have instance_size == 0, so we can use > it to set abstract=true on type_initialize(). > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > Changes v1 -> v2: > * Use old-fashioned if statement instead of "|=" on bool field > Suggested-by: Andreas Färber <afaerber@suse.de> > * Keep "device/introspect" path prefix on unit test > * Suggested-by: Andreas Färber <afaerber@suse.de> > > Changes v2 -> v3: > * Instead of making interfaces implicitly abstract, > set abstract=true explicitly for all of them and replace > type_initialize() abstract=true code with an assert(). > Suggested-by: Markus Armbruster <armbru@redhat.com> > * Try to make test code a bit more clear > --- > hw/acpi/acpi_interface.c | 1 + > hw/arm/boot.c | 1 + > hw/core/fw-path-provider.c | 1 + > hw/core/hotplug.c | 1 + > hw/core/nmi.c | 1 + > hw/core/stream.c | 1 + > hw/intc/intc.c | 1 + > hw/ipmi/ipmi.c | 1 + > hw/isa/isa-bus.c | 1 + > hw/ppc/pnv_xscom.c | 1 + > hw/timer/m48t59.c | 1 + > qom/object.c | 2 ++ > qom/object_interfaces.c | 1 + > tests/check-qom-interface.c | 1 + > tests/device-introspect-test.c | 58 +++++++++++++++++++++++++++++++++++++++--- > 15 files changed, 70 insertions(+), 3 deletions(-) > > diff --git a/hw/acpi/acpi_interface.c b/hw/acpi/acpi_interface.c > index 6583917..49388a9 100644 > --- a/hw/acpi/acpi_interface.c > +++ b/hw/acpi/acpi_interface.c > @@ -16,6 +16,7 @@ static void register_types(void) > static const TypeInfo acpi_dev_if_info = { > .name = TYPE_ACPI_DEVICE_IF, > .parent = TYPE_INTERFACE, > + .abstract = true, > .class_size = sizeof(AcpiDeviceIfClass), > }; > > diff --git a/hw/arm/boot.c b/hw/arm/boot.c > index ff621e4..65c59b5 100644 > --- a/hw/arm/boot.c > +++ b/hw/arm/boot.c > @@ -1031,6 +1031,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) > static const TypeInfo arm_linux_boot_if_info = { > .name = TYPE_ARM_LINUX_BOOT_IF, > .parent = TYPE_INTERFACE, > + .abstract = true, > .class_size = sizeof(ARMLinuxBootIfClass), > }; > > diff --git a/hw/core/fw-path-provider.c b/hw/core/fw-path-provider.c > index 33b9983..96f3131 100644 > --- a/hw/core/fw-path-provider.c > +++ b/hw/core/fw-path-provider.c > @@ -42,6 +42,7 @@ char *fw_path_provider_try_get_dev_path(Object *o, BusState *bus, > static const TypeInfo fw_path_provider_info = { > .name = TYPE_FW_PATH_PROVIDER, > .parent = TYPE_INTERFACE, > + .abstract = true, > .class_size = sizeof(FWPathProviderClass), > }; > > diff --git a/hw/core/hotplug.c b/hw/core/hotplug.c > index 17ac986..f74fba5 100644 > --- a/hw/core/hotplug.c > +++ b/hw/core/hotplug.c > @@ -60,6 +60,7 @@ void hotplug_handler_unplug(HotplugHandler *plug_handler, > static const TypeInfo hotplug_handler_info = { > .name = TYPE_HOTPLUG_HANDLER, > .parent = TYPE_INTERFACE, > + .abstract = true, > .class_size = sizeof(HotplugHandlerClass), > }; > > diff --git a/hw/core/nmi.c b/hw/core/nmi.c > index bfd0896..403edd3 100644 > --- a/hw/core/nmi.c > +++ b/hw/core/nmi.c > @@ -76,6 +76,7 @@ void nmi_monitor_handle(int cpu_index, Error **errp) > static const TypeInfo nmi_info = { > .name = TYPE_NMI, > .parent = TYPE_INTERFACE, > + .abstract = true, > .class_size = sizeof(NMIClass), > }; > > diff --git a/hw/core/stream.c b/hw/core/stream.c > index 4439ecd..db7f90b 100644 > --- a/hw/core/stream.c > +++ b/hw/core/stream.c > @@ -21,6 +21,7 @@ stream_can_push(StreamSlave *sink, StreamCanPushNotifyFn notify, > static const TypeInfo stream_slave_info = { > .name = TYPE_STREAM_SLAVE, > .parent = TYPE_INTERFACE, > + .abstract = true, > .class_size = sizeof(StreamSlaveClass), > }; > > diff --git a/hw/intc/intc.c b/hw/intc/intc.c > index 2e1e29e..9373546 100644 > --- a/hw/intc/intc.c > +++ b/hw/intc/intc.c > @@ -29,6 +29,7 @@ > static const TypeInfo intctrl_info = { > .name = TYPE_INTERRUPT_STATS_PROVIDER, > .parent = TYPE_INTERFACE, > + .abstract = true, > .class_size = sizeof(InterruptStatsProviderClass), > }; > > diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c > index 5cf1caa..c255499 100644 > --- a/hw/ipmi/ipmi.c > +++ b/hw/ipmi/ipmi.c > @@ -86,6 +86,7 @@ static void ipmi_interface_class_init(ObjectClass *class, void *data) > static TypeInfo ipmi_interface_type_info = { > .name = TYPE_IPMI_INTERFACE, > .parent = TYPE_INTERFACE, > + .abstract = true, > .class_size = sizeof(IPMIInterfaceClass), > .class_init = ipmi_interface_class_init, > }; > diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c > index 9d07b11..a1cad86 100644 > --- a/hw/isa/isa-bus.c > +++ b/hw/isa/isa-bus.c > @@ -41,6 +41,7 @@ static void isa_bus_class_init(ObjectClass *klass, void *data) > static const TypeInfo isa_dma_info = { > .name = TYPE_ISADMA, > .parent = TYPE_INTERFACE, > + .abstract = true, > .class_size = sizeof(IsaDmaClass), > }; > > diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c > index 8da2718..642f104 100644 > --- a/hw/ppc/pnv_xscom.c > +++ b/hw/ppc/pnv_xscom.c > @@ -192,6 +192,7 @@ void pnv_xscom_realize(PnvChip *chip, Error **errp) > static const TypeInfo pnv_xscom_interface_info = { > .name = TYPE_PNV_XSCOM_INTERFACE, > .parent = TYPE_INTERFACE, > + .abstract = true, > .class_size = sizeof(PnvXScomInterfaceClass), > }; > > diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c > index e46ca88..42c5fc7 100644 > --- a/hw/timer/m48t59.c > +++ b/hw/timer/m48t59.c > @@ -884,6 +884,7 @@ static void m48txx_sysbus_concrete_class_init(ObjectClass *klass, void *data) > static const TypeInfo nvram_info = { > .name = TYPE_NVRAM, > .parent = TYPE_INTERFACE, > + .abstract = true, > .class_size = sizeof(NvramClass), > }; > > diff --git a/qom/object.c b/qom/object.c > index 7a05e35..d93989a 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -272,6 +272,8 @@ static void type_initialize(TypeImpl *ti) > > ti->class_size = type_class_get_size(ti); > ti->instance_size = type_object_get_size(ti); > + /* types with zero instance_size can't be instantiated, must be abstract */ > + assert(ti->instance_size > 0 || ti->abstract); > > ti->class = g_malloc0(ti->class_size); > Separate idea: should type_initialize_interface() check !interface_type->instance_size? > diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c > index ded4d84..f6dc46c 100644 > --- a/qom/object_interfaces.c > +++ b/qom/object_interfaces.c > @@ -217,6 +217,7 @@ static void register_types(void) > static const TypeInfo uc_interface_info = { > .name = TYPE_USER_CREATABLE, > .parent = TYPE_INTERFACE, > + .abstract = true, > .class_size = sizeof(UserCreatableClass), > }; > > diff --git a/tests/check-qom-interface.c b/tests/check-qom-interface.c > index f87c9aa..560b7e2 100644 > --- a/tests/check-qom-interface.c > +++ b/tests/check-qom-interface.c > @@ -36,6 +36,7 @@ typedef struct TestIfClass { > static const TypeInfo test_if_info = { > .name = TYPE_TEST_IF, > .parent = TYPE_INTERFACE, > + .abstract = true, > .class_size = sizeof(TestIfClass), > }; > Just 13 .abstract = true; fairly harmless. > diff --git a/tests/device-introspect-test.c b/tests/device-introspect-test.c > index 37debc1..4fa8d29 100644 > --- a/tests/device-introspect-test.c > +++ b/tests/device-introspect-test.c > @@ -20,18 +20,24 @@ > #include "qemu/osdep.h" > #include "qemu-common.h" > #include "qapi/qmp/qstring.h" > +#include "qapi/qmp/qbool.h" > +#include "qapi/qmp/qdict.h" > #include "libqtest.h" > > const char common_args[] = "-nodefaults -machine none"; > > -static QList *device_type_list(bool abstract) > +static QList *qom_list_types(const char *implements, bool abstract) > { > QDict *resp; > QList *ret; > + QDict *args = qdict_new(); > > + qdict_put(args, "abstract", qbool_from_bool(abstract)); > + if (implements) { > + qdict_put(args, "implements", qstring_from_str(implements)); > + } > resp = qmp("{'execute': 'qom-list-types'," > - " 'arguments': {'implements': 'device', 'abstract': %i}}", > - abstract); > + " 'arguments': %p }", args); > g_assert(qdict_haskey(resp, "return")); > ret = qdict_get_qlist(resp, "return"); > QINCREF(ret); > @@ -39,6 +45,11 @@ static QList *device_type_list(bool abstract) > return ret; > } > > +static QList *device_type_list(bool abstract) > +{ > + return qom_list_types("device", abstract); > +} > + > static void test_one_device(const char *type) > { > QDict *resp; > @@ -110,6 +121,46 @@ static void test_device_intro_concrete(void) > qtest_end(); > } > > +static void test_abstract_interfaces(void) > +{ > + QList *all_types; > + QList *obj_types; > + QListEntry *ae; > + > + qtest_start(common_args); > + /* qom-list-types implements=interface would return any type > + * that implements _any_ interface (not just interface types), > + * so use a trick to find the interface type names: > + * 1) list all non-abstract types > + * 2) list all non-abstract object types > + * 3) Look for items in (1) that are not in (2) > + */ > + all_types = qom_list_types(NULL, false); All concrete types. > + obj_types = qom_list_types("object", false); All concrete subtypes of "object". > + > + QLIST_FOREACH_ENTRY(all_types, ae) { For @at in concrete types: > + QDict *at = qobject_to_qdict(qlist_entry_obj(ae)); > + const char *aname = qdict_get_str(at, "name"); > + QListEntry *oe; > + const char *found = NULL; > + Search the list of concrete subtypes of "object" for @at > + QLIST_FOREACH_ENTRY(obj_types, oe) { > + QDict *ot = qobject_to_qdict(qlist_entry_obj(oe)); > + const char *oname = qdict_get_str(ot, "name"); > + if (!strcmp(aname, oname)) { > + found = oname; > + break; > + } > + } > + > + g_assert(found); Assert @at is a concrete subtype of "object". Taken together: assert that any concrete type is a subtype of "object". Correct? How's that related to interfaces? > + } > + > + QDECREF(all_types); > + QDECREF(obj_types); > + qtest_end(); > +} > + > int main(int argc, char **argv) > { > g_test_init(&argc, &argv, NULL); > @@ -118,6 +169,7 @@ int main(int argc, char **argv) > qtest_add_func("device/introspect/none", test_device_intro_none); > qtest_add_func("device/introspect/abstract", test_device_intro_abstract); > qtest_add_func("device/introspect/concrete", test_device_intro_concrete); > + qtest_add_func("device/introspect/abstract-interfaces", test_abstract_interfaces); > > return g_test_run(); > }
On Thu, Dec 15, 2016 at 10:17:27AM +0100, Markus Armbruster wrote: > Eduardo Habkost <ehabkost@redhat.com> writes: > > > "qom-list-types abstract=false" currently returns all interface > > types, as if they were not abstract. Fix this by making sure all > > interface types are abstract. > > > > All interface types have instance_size == 0, so we can use > > it to set abstract=true on type_initialize(). > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > --- > > Changes v1 -> v2: > > * Use old-fashioned if statement instead of "|=" on bool field > > Suggested-by: Andreas Färber <afaerber@suse.de> > > * Keep "device/introspect" path prefix on unit test > > * Suggested-by: Andreas Färber <afaerber@suse.de> > > > > Changes v2 -> v3: > > * Instead of making interfaces implicitly abstract, > > set abstract=true explicitly for all of them and replace > > type_initialize() abstract=true code with an assert(). > > Suggested-by: Markus Armbruster <armbru@redhat.com> > > * Try to make test code a bit more clear > > --- > > hw/acpi/acpi_interface.c | 1 + > > hw/arm/boot.c | 1 + > > hw/core/fw-path-provider.c | 1 + > > hw/core/hotplug.c | 1 + > > hw/core/nmi.c | 1 + > > hw/core/stream.c | 1 + > > hw/intc/intc.c | 1 + > > hw/ipmi/ipmi.c | 1 + > > hw/isa/isa-bus.c | 1 + > > hw/ppc/pnv_xscom.c | 1 + > > hw/timer/m48t59.c | 1 + > > qom/object.c | 2 ++ > > qom/object_interfaces.c | 1 + > > tests/check-qom-interface.c | 1 + > > tests/device-introspect-test.c | 58 +++++++++++++++++++++++++++++++++++++++--- > > 15 files changed, 70 insertions(+), 3 deletions(-) > > > > diff --git a/hw/acpi/acpi_interface.c b/hw/acpi/acpi_interface.c > > index 6583917..49388a9 100644 > > --- a/hw/acpi/acpi_interface.c > > +++ b/hw/acpi/acpi_interface.c > > @@ -16,6 +16,7 @@ static void register_types(void) > > static const TypeInfo acpi_dev_if_info = { > > .name = TYPE_ACPI_DEVICE_IF, > > .parent = TYPE_INTERFACE, > > + .abstract = true, > > .class_size = sizeof(AcpiDeviceIfClass), > > }; > > > > diff --git a/hw/arm/boot.c b/hw/arm/boot.c > > index ff621e4..65c59b5 100644 > > --- a/hw/arm/boot.c > > +++ b/hw/arm/boot.c > > @@ -1031,6 +1031,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) > > static const TypeInfo arm_linux_boot_if_info = { > > .name = TYPE_ARM_LINUX_BOOT_IF, > > .parent = TYPE_INTERFACE, > > + .abstract = true, > > .class_size = sizeof(ARMLinuxBootIfClass), > > }; > > > > diff --git a/hw/core/fw-path-provider.c b/hw/core/fw-path-provider.c > > index 33b9983..96f3131 100644 > > --- a/hw/core/fw-path-provider.c > > +++ b/hw/core/fw-path-provider.c > > @@ -42,6 +42,7 @@ char *fw_path_provider_try_get_dev_path(Object *o, BusState *bus, > > static const TypeInfo fw_path_provider_info = { > > .name = TYPE_FW_PATH_PROVIDER, > > .parent = TYPE_INTERFACE, > > + .abstract = true, > > .class_size = sizeof(FWPathProviderClass), > > }; > > > > diff --git a/hw/core/hotplug.c b/hw/core/hotplug.c > > index 17ac986..f74fba5 100644 > > --- a/hw/core/hotplug.c > > +++ b/hw/core/hotplug.c > > @@ -60,6 +60,7 @@ void hotplug_handler_unplug(HotplugHandler *plug_handler, > > static const TypeInfo hotplug_handler_info = { > > .name = TYPE_HOTPLUG_HANDLER, > > .parent = TYPE_INTERFACE, > > + .abstract = true, > > .class_size = sizeof(HotplugHandlerClass), > > }; > > > > diff --git a/hw/core/nmi.c b/hw/core/nmi.c > > index bfd0896..403edd3 100644 > > --- a/hw/core/nmi.c > > +++ b/hw/core/nmi.c > > @@ -76,6 +76,7 @@ void nmi_monitor_handle(int cpu_index, Error **errp) > > static const TypeInfo nmi_info = { > > .name = TYPE_NMI, > > .parent = TYPE_INTERFACE, > > + .abstract = true, > > .class_size = sizeof(NMIClass), > > }; > > > > diff --git a/hw/core/stream.c b/hw/core/stream.c > > index 4439ecd..db7f90b 100644 > > --- a/hw/core/stream.c > > +++ b/hw/core/stream.c > > @@ -21,6 +21,7 @@ stream_can_push(StreamSlave *sink, StreamCanPushNotifyFn notify, > > static const TypeInfo stream_slave_info = { > > .name = TYPE_STREAM_SLAVE, > > .parent = TYPE_INTERFACE, > > + .abstract = true, > > .class_size = sizeof(StreamSlaveClass), > > }; > > > > diff --git a/hw/intc/intc.c b/hw/intc/intc.c > > index 2e1e29e..9373546 100644 > > --- a/hw/intc/intc.c > > +++ b/hw/intc/intc.c > > @@ -29,6 +29,7 @@ > > static const TypeInfo intctrl_info = { > > .name = TYPE_INTERRUPT_STATS_PROVIDER, > > .parent = TYPE_INTERFACE, > > + .abstract = true, > > .class_size = sizeof(InterruptStatsProviderClass), > > }; > > > > diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c > > index 5cf1caa..c255499 100644 > > --- a/hw/ipmi/ipmi.c > > +++ b/hw/ipmi/ipmi.c > > @@ -86,6 +86,7 @@ static void ipmi_interface_class_init(ObjectClass *class, void *data) > > static TypeInfo ipmi_interface_type_info = { > > .name = TYPE_IPMI_INTERFACE, > > .parent = TYPE_INTERFACE, > > + .abstract = true, > > .class_size = sizeof(IPMIInterfaceClass), > > .class_init = ipmi_interface_class_init, > > }; > > diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c > > index 9d07b11..a1cad86 100644 > > --- a/hw/isa/isa-bus.c > > +++ b/hw/isa/isa-bus.c > > @@ -41,6 +41,7 @@ static void isa_bus_class_init(ObjectClass *klass, void *data) > > static const TypeInfo isa_dma_info = { > > .name = TYPE_ISADMA, > > .parent = TYPE_INTERFACE, > > + .abstract = true, > > .class_size = sizeof(IsaDmaClass), > > }; > > > > diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c > > index 8da2718..642f104 100644 > > --- a/hw/ppc/pnv_xscom.c > > +++ b/hw/ppc/pnv_xscom.c > > @@ -192,6 +192,7 @@ void pnv_xscom_realize(PnvChip *chip, Error **errp) > > static const TypeInfo pnv_xscom_interface_info = { > > .name = TYPE_PNV_XSCOM_INTERFACE, > > .parent = TYPE_INTERFACE, > > + .abstract = true, > > .class_size = sizeof(PnvXScomInterfaceClass), > > }; > > > > diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c > > index e46ca88..42c5fc7 100644 > > --- a/hw/timer/m48t59.c > > +++ b/hw/timer/m48t59.c > > @@ -884,6 +884,7 @@ static void m48txx_sysbus_concrete_class_init(ObjectClass *klass, void *data) > > static const TypeInfo nvram_info = { > > .name = TYPE_NVRAM, > > .parent = TYPE_INTERFACE, > > + .abstract = true, > > .class_size = sizeof(NvramClass), > > }; > > > > diff --git a/qom/object.c b/qom/object.c > > index 7a05e35..d93989a 100644 > > --- a/qom/object.c > > +++ b/qom/object.c > > @@ -272,6 +272,8 @@ static void type_initialize(TypeImpl *ti) > > > > ti->class_size = type_class_get_size(ti); > > ti->instance_size = type_object_get_size(ti); > > + /* types with zero instance_size can't be instantiated, must be abstract */ > > + assert(ti->instance_size > 0 || ti->abstract); > > > > ti->class = g_malloc0(ti->class_size); > > > > Separate idea: should type_initialize_interface() check > !interface_type->instance_size? > > > diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c > > index ded4d84..f6dc46c 100644 > > --- a/qom/object_interfaces.c > > +++ b/qom/object_interfaces.c > > @@ -217,6 +217,7 @@ static void register_types(void) > > static const TypeInfo uc_interface_info = { > > .name = TYPE_USER_CREATABLE, > > .parent = TYPE_INTERFACE, > > + .abstract = true, > > .class_size = sizeof(UserCreatableClass), > > }; > > > > diff --git a/tests/check-qom-interface.c b/tests/check-qom-interface.c > > index f87c9aa..560b7e2 100644 > > --- a/tests/check-qom-interface.c > > +++ b/tests/check-qom-interface.c > > @@ -36,6 +36,7 @@ typedef struct TestIfClass { > > static const TypeInfo test_if_info = { > > .name = TYPE_TEST_IF, > > .parent = TYPE_INTERFACE, > > + .abstract = true, > > .class_size = sizeof(TestIfClass), > > }; > > > > Just 13 .abstract = true; fairly harmless. > > > diff --git a/tests/device-introspect-test.c b/tests/device-introspect-test.c > > index 37debc1..4fa8d29 100644 > > --- a/tests/device-introspect-test.c > > +++ b/tests/device-introspect-test.c > > @@ -20,18 +20,24 @@ > > #include "qemu/osdep.h" > > #include "qemu-common.h" > > #include "qapi/qmp/qstring.h" > > +#include "qapi/qmp/qbool.h" > > +#include "qapi/qmp/qdict.h" > > #include "libqtest.h" > > > > const char common_args[] = "-nodefaults -machine none"; > > > > -static QList *device_type_list(bool abstract) > > +static QList *qom_list_types(const char *implements, bool abstract) > > { > > QDict *resp; > > QList *ret; > > + QDict *args = qdict_new(); > > > > + qdict_put(args, "abstract", qbool_from_bool(abstract)); > > + if (implements) { > > + qdict_put(args, "implements", qstring_from_str(implements)); > > + } > > resp = qmp("{'execute': 'qom-list-types'," > > - " 'arguments': {'implements': 'device', 'abstract': %i}}", > > - abstract); > > + " 'arguments': %p }", args); > > g_assert(qdict_haskey(resp, "return")); > > ret = qdict_get_qlist(resp, "return"); > > QINCREF(ret); > > @@ -39,6 +45,11 @@ static QList *device_type_list(bool abstract) > > return ret; > > } > > > > +static QList *device_type_list(bool abstract) > > +{ > > + return qom_list_types("device", abstract); > > +} > > + > > static void test_one_device(const char *type) > > { > > QDict *resp; > > @@ -110,6 +121,46 @@ static void test_device_intro_concrete(void) > > qtest_end(); > > } > > > > +static void test_abstract_interfaces(void) > > +{ > > + QList *all_types; > > + QList *obj_types; > > + QListEntry *ae; > > + > > + qtest_start(common_args); > > + /* qom-list-types implements=interface would return any type > > + * that implements _any_ interface (not just interface types), > > + * so use a trick to find the interface type names: > > + * 1) list all non-abstract types > > + * 2) list all non-abstract object types > > + * 3) Look for items in (1) that are not in (2) > > + */ > > + all_types = qom_list_types(NULL, false); > > All concrete types. > > > + obj_types = qom_list_types("object", false); > > All concrete subtypes of "object". > > > + > > + QLIST_FOREACH_ENTRY(all_types, ae) { > > For @at in concrete types: > > > + QDict *at = qobject_to_qdict(qlist_entry_obj(ae)); > > + const char *aname = qdict_get_str(at, "name"); > > + QListEntry *oe; > > + const char *found = NULL; > > + > > Search the list of concrete subtypes of "object" for @at > > > + QLIST_FOREACH_ENTRY(obj_types, oe) { > > + QDict *ot = qobject_to_qdict(qlist_entry_obj(oe)); > > + const char *oname = qdict_get_str(ot, "name"); > > + if (!strcmp(aname, oname)) { > > + found = oname; > > + break; > > + } > > + } > > + > > + g_assert(found); > > Assert @at is a concrete subtype of "object". > > Taken together: assert that any concrete type is a subtype of "object". > Correct? > > How's that related to interfaces? The only types that are not subtypes of "object" are interfaces. If we see any concrete type that is not subtype of "object", it is a non-abstract interface (or something even weirder). Anyway, I believe this test case is complete unnecessary in v3. It made sense to make sure I was adding the automatic abstract=true line at the right place in the code. Now we have explicitly changed all interfaces to have abstract=true, and added an assert() to the code. This test code can be removed from the patch. > > > + } > > + > > + QDECREF(all_types); > > + QDECREF(obj_types); > > + qtest_end(); > > +} > > + > > int main(int argc, char **argv) > > { > > g_test_init(&argc, &argv, NULL); > > @@ -118,6 +169,7 @@ int main(int argc, char **argv) > > qtest_add_func("device/introspect/none", test_device_intro_none); > > qtest_add_func("device/introspect/abstract", test_device_intro_abstract); > > qtest_add_func("device/introspect/concrete", test_device_intro_concrete); > > + qtest_add_func("device/introspect/abstract-interfaces", test_abstract_interfaces); > > > > return g_test_run(); > > }
diff --git a/hw/acpi/acpi_interface.c b/hw/acpi/acpi_interface.c index 6583917..49388a9 100644 --- a/hw/acpi/acpi_interface.c +++ b/hw/acpi/acpi_interface.c @@ -16,6 +16,7 @@ static void register_types(void) static const TypeInfo acpi_dev_if_info = { .name = TYPE_ACPI_DEVICE_IF, .parent = TYPE_INTERFACE, + .abstract = true, .class_size = sizeof(AcpiDeviceIfClass), }; diff --git a/hw/arm/boot.c b/hw/arm/boot.c index ff621e4..65c59b5 100644 --- a/hw/arm/boot.c +++ b/hw/arm/boot.c @@ -1031,6 +1031,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) static const TypeInfo arm_linux_boot_if_info = { .name = TYPE_ARM_LINUX_BOOT_IF, .parent = TYPE_INTERFACE, + .abstract = true, .class_size = sizeof(ARMLinuxBootIfClass), }; diff --git a/hw/core/fw-path-provider.c b/hw/core/fw-path-provider.c index 33b9983..96f3131 100644 --- a/hw/core/fw-path-provider.c +++ b/hw/core/fw-path-provider.c @@ -42,6 +42,7 @@ char *fw_path_provider_try_get_dev_path(Object *o, BusState *bus, static const TypeInfo fw_path_provider_info = { .name = TYPE_FW_PATH_PROVIDER, .parent = TYPE_INTERFACE, + .abstract = true, .class_size = sizeof(FWPathProviderClass), }; diff --git a/hw/core/hotplug.c b/hw/core/hotplug.c index 17ac986..f74fba5 100644 --- a/hw/core/hotplug.c +++ b/hw/core/hotplug.c @@ -60,6 +60,7 @@ void hotplug_handler_unplug(HotplugHandler *plug_handler, static const TypeInfo hotplug_handler_info = { .name = TYPE_HOTPLUG_HANDLER, .parent = TYPE_INTERFACE, + .abstract = true, .class_size = sizeof(HotplugHandlerClass), }; diff --git a/hw/core/nmi.c b/hw/core/nmi.c index bfd0896..403edd3 100644 --- a/hw/core/nmi.c +++ b/hw/core/nmi.c @@ -76,6 +76,7 @@ void nmi_monitor_handle(int cpu_index, Error **errp) static const TypeInfo nmi_info = { .name = TYPE_NMI, .parent = TYPE_INTERFACE, + .abstract = true, .class_size = sizeof(NMIClass), }; diff --git a/hw/core/stream.c b/hw/core/stream.c index 4439ecd..db7f90b 100644 --- a/hw/core/stream.c +++ b/hw/core/stream.c @@ -21,6 +21,7 @@ stream_can_push(StreamSlave *sink, StreamCanPushNotifyFn notify, static const TypeInfo stream_slave_info = { .name = TYPE_STREAM_SLAVE, .parent = TYPE_INTERFACE, + .abstract = true, .class_size = sizeof(StreamSlaveClass), }; diff --git a/hw/intc/intc.c b/hw/intc/intc.c index 2e1e29e..9373546 100644 --- a/hw/intc/intc.c +++ b/hw/intc/intc.c @@ -29,6 +29,7 @@ static const TypeInfo intctrl_info = { .name = TYPE_INTERRUPT_STATS_PROVIDER, .parent = TYPE_INTERFACE, + .abstract = true, .class_size = sizeof(InterruptStatsProviderClass), }; diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c index 5cf1caa..c255499 100644 --- a/hw/ipmi/ipmi.c +++ b/hw/ipmi/ipmi.c @@ -86,6 +86,7 @@ static void ipmi_interface_class_init(ObjectClass *class, void *data) static TypeInfo ipmi_interface_type_info = { .name = TYPE_IPMI_INTERFACE, .parent = TYPE_INTERFACE, + .abstract = true, .class_size = sizeof(IPMIInterfaceClass), .class_init = ipmi_interface_class_init, }; diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c index 9d07b11..a1cad86 100644 --- a/hw/isa/isa-bus.c +++ b/hw/isa/isa-bus.c @@ -41,6 +41,7 @@ static void isa_bus_class_init(ObjectClass *klass, void *data) static const TypeInfo isa_dma_info = { .name = TYPE_ISADMA, .parent = TYPE_INTERFACE, + .abstract = true, .class_size = sizeof(IsaDmaClass), }; diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c index 8da2718..642f104 100644 --- a/hw/ppc/pnv_xscom.c +++ b/hw/ppc/pnv_xscom.c @@ -192,6 +192,7 @@ void pnv_xscom_realize(PnvChip *chip, Error **errp) static const TypeInfo pnv_xscom_interface_info = { .name = TYPE_PNV_XSCOM_INTERFACE, .parent = TYPE_INTERFACE, + .abstract = true, .class_size = sizeof(PnvXScomInterfaceClass), }; diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c index e46ca88..42c5fc7 100644 --- a/hw/timer/m48t59.c +++ b/hw/timer/m48t59.c @@ -884,6 +884,7 @@ static void m48txx_sysbus_concrete_class_init(ObjectClass *klass, void *data) static const TypeInfo nvram_info = { .name = TYPE_NVRAM, .parent = TYPE_INTERFACE, + .abstract = true, .class_size = sizeof(NvramClass), }; diff --git a/qom/object.c b/qom/object.c index 7a05e35..d93989a 100644 --- a/qom/object.c +++ b/qom/object.c @@ -272,6 +272,8 @@ static void type_initialize(TypeImpl *ti) ti->class_size = type_class_get_size(ti); ti->instance_size = type_object_get_size(ti); + /* types with zero instance_size can't be instantiated, must be abstract */ + assert(ti->instance_size > 0 || ti->abstract); ti->class = g_malloc0(ti->class_size); diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c index ded4d84..f6dc46c 100644 --- a/qom/object_interfaces.c +++ b/qom/object_interfaces.c @@ -217,6 +217,7 @@ static void register_types(void) static const TypeInfo uc_interface_info = { .name = TYPE_USER_CREATABLE, .parent = TYPE_INTERFACE, + .abstract = true, .class_size = sizeof(UserCreatableClass), }; diff --git a/tests/check-qom-interface.c b/tests/check-qom-interface.c index f87c9aa..560b7e2 100644 --- a/tests/check-qom-interface.c +++ b/tests/check-qom-interface.c @@ -36,6 +36,7 @@ typedef struct TestIfClass { static const TypeInfo test_if_info = { .name = TYPE_TEST_IF, .parent = TYPE_INTERFACE, + .abstract = true, .class_size = sizeof(TestIfClass), }; diff --git a/tests/device-introspect-test.c b/tests/device-introspect-test.c index 37debc1..4fa8d29 100644 --- a/tests/device-introspect-test.c +++ b/tests/device-introspect-test.c @@ -20,18 +20,24 @@ #include "qemu/osdep.h" #include "qemu-common.h" #include "qapi/qmp/qstring.h" +#include "qapi/qmp/qbool.h" +#include "qapi/qmp/qdict.h" #include "libqtest.h" const char common_args[] = "-nodefaults -machine none"; -static QList *device_type_list(bool abstract) +static QList *qom_list_types(const char *implements, bool abstract) { QDict *resp; QList *ret; + QDict *args = qdict_new(); + qdict_put(args, "abstract", qbool_from_bool(abstract)); + if (implements) { + qdict_put(args, "implements", qstring_from_str(implements)); + } resp = qmp("{'execute': 'qom-list-types'," - " 'arguments': {'implements': 'device', 'abstract': %i}}", - abstract); + " 'arguments': %p }", args); g_assert(qdict_haskey(resp, "return")); ret = qdict_get_qlist(resp, "return"); QINCREF(ret); @@ -39,6 +45,11 @@ static QList *device_type_list(bool abstract) return ret; } +static QList *device_type_list(bool abstract) +{ + return qom_list_types("device", abstract); +} + static void test_one_device(const char *type) { QDict *resp; @@ -110,6 +121,46 @@ static void test_device_intro_concrete(void) qtest_end(); } +static void test_abstract_interfaces(void) +{ + QList *all_types; + QList *obj_types; + QListEntry *ae; + + qtest_start(common_args); + /* qom-list-types implements=interface would return any type + * that implements _any_ interface (not just interface types), + * so use a trick to find the interface type names: + * 1) list all non-abstract types + * 2) list all non-abstract object types + * 3) Look for items in (1) that are not in (2) + */ + all_types = qom_list_types(NULL, false); + obj_types = qom_list_types("object", false); + + QLIST_FOREACH_ENTRY(all_types, ae) { + QDict *at = qobject_to_qdict(qlist_entry_obj(ae)); + const char *aname = qdict_get_str(at, "name"); + QListEntry *oe; + const char *found = NULL; + + QLIST_FOREACH_ENTRY(obj_types, oe) { + QDict *ot = qobject_to_qdict(qlist_entry_obj(oe)); + const char *oname = qdict_get_str(ot, "name"); + if (!strcmp(aname, oname)) { + found = oname; + break; + } + } + + g_assert(found); + } + + QDECREF(all_types); + QDECREF(obj_types); + qtest_end(); +} + int main(int argc, char **argv) { g_test_init(&argc, &argv, NULL); @@ -118,6 +169,7 @@ int main(int argc, char **argv) qtest_add_func("device/introspect/none", test_device_intro_none); qtest_add_func("device/introspect/abstract", test_device_intro_abstract); qtest_add_func("device/introspect/concrete", test_device_intro_concrete); + qtest_add_func("device/introspect/abstract-interfaces", test_abstract_interfaces); return g_test_run(); }
"qom-list-types abstract=false" currently returns all interface types, as if they were not abstract. Fix this by making sure all interface types are abstract. All interface types have instance_size == 0, so we can use it to set abstract=true on type_initialize(). Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Changes v1 -> v2: * Use old-fashioned if statement instead of "|=" on bool field Suggested-by: Andreas Färber <afaerber@suse.de> * Keep "device/introspect" path prefix on unit test * Suggested-by: Andreas Färber <afaerber@suse.de> Changes v2 -> v3: * Instead of making interfaces implicitly abstract, set abstract=true explicitly for all of them and replace type_initialize() abstract=true code with an assert(). Suggested-by: Markus Armbruster <armbru@redhat.com> * Try to make test code a bit more clear --- hw/acpi/acpi_interface.c | 1 + hw/arm/boot.c | 1 + hw/core/fw-path-provider.c | 1 + hw/core/hotplug.c | 1 + hw/core/nmi.c | 1 + hw/core/stream.c | 1 + hw/intc/intc.c | 1 + hw/ipmi/ipmi.c | 1 + hw/isa/isa-bus.c | 1 + hw/ppc/pnv_xscom.c | 1 + hw/timer/m48t59.c | 1 + qom/object.c | 2 ++ qom/object_interfaces.c | 1 + tests/check-qom-interface.c | 1 + tests/device-introspect-test.c | 58 +++++++++++++++++++++++++++++++++++++++--- 15 files changed, 70 insertions(+), 3 deletions(-)