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?
>
>
> >>
> >> 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)
> >> {
> >
> >
>
>
>
