Hi On Fri, Jul 29, 2016 at 10:31 AM, Igor Mammedov <imamm...@redhat.com> wrote: > On Thu, 28 Jul 2016 17:43:37 +0400 > Marc-André Lureau <marcandre.lur...@gmail.com> wrote: > >> Hi >> >> On Thu, Jul 28, 2016 at 3:29 PM, Igor Mammedov <imamm...@redhat.com> wrote: >> > On Thu, 28 Jul 2016 15:13:57 +0400 >> > marcandre.lur...@redhat.com wrote: >> > >> >> From: Marc-André Lureau <marcandre.lur...@redhat.com> >> >> >> >> The property should own the allocated and unreferenced pointer. In case >> >> of error, it should also be freed. >> > I wonder, what use case triggers above error >> > >> > >> >> See right below in commit message: >> >> /x86_64/qom/pc-i440fx-1.7: qemu-system-x86_64: attempt to add >> duplicate property 'acpi-pcihp-bsel' to object (type 'PCI') > Maybe I've should have asked in another way, > How do you reproduce it?
It's a qtest error (with the error reporting added here): QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/qom-test -p /x86_64/qom/pc-i440fx-1.7 This is also triggered simply by running: x86_64-softmmu/qemu-system-x86_64 -machine pc-i440fx-1.7 > >> >> >> >> >> >> RFC, because this patch triggers: >> >> /x86_64/qom/pc-i440fx-1.7: >> >> qemu-system-x86_64: attempt to add duplicate property 'acpi-pcihp-bsel' >> >> to object (type 'PCI') >> >> >> >> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> >> >> --- >> >> hw/i386/acpi-build.c | 15 +++++++++++++-- >> >> include/qom/object.h | 4 ++++ >> >> qom/object.c | 9 +++++++++ >> >> 3 files changed, 26 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c >> >> index 017bb51..2012007 100644 >> >> --- a/hw/i386/acpi-build.c >> >> +++ b/hw/i386/acpi-build.c >> >> @@ -425,6 +425,11 @@ build_madt(GArray *table_data, BIOSLinker *linker, >> >> PCMachineState *pcms) >> >> table_data->len - madt_start, 1, NULL, NULL); >> >> } >> >> >> >> +static void bsel_release(Object *obj, const char *name, void *opaque) >> >> +{ >> >> + g_free(opaque); >> >> +} >> >> + >> >> /* Assign BSEL property to all buses. In the future, this can be changed >> >> * to only assign to buses that support hotplug. >> >> */ >> >> @@ -432,13 +437,19 @@ static void *acpi_set_bsel(PCIBus *bus, void >> >> *opaque) >> >> { >> >> unsigned *bsel_alloc = opaque; >> >> unsigned *bus_bsel; >> >> + Error *err = NULL; >> >> >> >> if (qbus_is_hotpluggable(BUS(bus))) { >> >> bus_bsel = g_malloc(sizeof *bus_bsel); >> >> >> >> *bus_bsel = (*bsel_alloc)++; >> >> - object_property_add_uint32_ptr(OBJECT(bus), ACPI_PCIHP_PROP_BSEL, >> >> - bus_bsel, NULL); >> >> + object_property_add_uint32_ptr_release(OBJECT(bus), >> >> + ACPI_PCIHP_PROP_BSEL, >> >> + bus_bsel, bsel_release, >> >> &err); >> >> + if (err) { >> >> + g_free(bus_bsel); >> >> + error_report_err(err); >> >> + } >> >> } >> >> >> >> return bsel_alloc; >> >> diff --git a/include/qom/object.h b/include/qom/object.h >> >> index 5ecc2d1..41c1051 100644 >> >> --- a/include/qom/object.h >> >> +++ b/include/qom/object.h >> >> @@ -1488,6 +1488,10 @@ void >> >> object_class_property_add_uint16_ptr(ObjectClass *klass, const char *name, >> >> */ >> >> void object_property_add_uint32_ptr(Object *obj, const char *name, >> >> const uint32_t *v, Error **errp); >> >> +void object_property_add_uint32_ptr_release(Object *obj, const char >> >> *name, >> >> + uint32_t *v, >> >> + ObjectPropertyRelease >> >> *release, >> >> + Error **errp); >> >> void object_class_property_add_uint32_ptr(ObjectClass *klass, const char >> >> *name, >> >> const uint32_t *v, Error >> >> **errp); >> >> >> >> diff --git a/qom/object.c b/qom/object.c >> >> index 8166b7d..1635f57 100644 >> >> --- a/qom/object.c >> >> +++ b/qom/object.c >> >> @@ -2157,6 +2157,15 @@ void object_property_add_uint32_ptr(Object *obj, >> >> const char *name, >> >> NULL, NULL, (void *)v, errp); >> >> } >> >> >> >> +void object_property_add_uint32_ptr_release(Object *obj, const char >> >> *name, >> >> + uint32_t *v, >> >> + ObjectPropertyRelease >> >> *release, >> >> + Error **errp) >> >> +{ >> >> + object_property_add(obj, name, "uint32", property_get_uint32_ptr, >> >> + NULL, release, (void *)v, errp); >> >> +} >> >> + >> >> void object_class_property_add_uint32_ptr(ObjectClass *klass, const char >> >> *name, >> >> const uint32_t *v, Error >> >> **errp) >> >> { >> > >> > >> >> >> > -- Marc-André Lureau