| Message ID | 20200204131601.125677-1-felipe@nutanix.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Improve default object property_add uint helpers | expand |
Hi On Tue, Feb 4, 2020 at 2:16 PM Felipe Franciosi <felipe@nutanix.com> wrote: > > This improves the family of object_property_add_uintXX_ptr helpers by enabling > a default getter/setter only when desired. To prevent an API behavioural change > (from clients that already used these helpers and did not want a setter), we > add a OBJ_PROP_FLAG_READ flag that allow clients to only have a getter. Patch 1 > enhances the API and modify current users. > > While modifying the clients of the API, a couple of improvement opportunities > were observed in ich9. These were added in separate patches (2 and 3). > > Patch 4 cleans up a lot of existing code by moving various objects to the > enhanced API. Previously, those objects had their own getters/setters that only > updated the values without further checks. Some of them actually lacked a check > for setting overflows, which could have resulted in undesired values being set. > The new default setters include a check for that, not updating the values in > case of errors (and propagating them). If they did not provide an error > pointer, then that behaviour was maintained. > > Felipe Franciosi (4): > qom/object: enable setter for uint types > ich9: fix getter type for sci_int property > ich9: Simplify ich9_lpc_initfn > qom/object: Use common get/set uint helpers > > hw/acpi/ich9.c | 99 ++------------------ > hw/acpi/pcihp.c | 7 +- > hw/acpi/piix4.c | 12 +-- > hw/isa/lpc_ich9.c | 27 ++---- > hw/misc/edu.c | 13 +-- > hw/pci-host/q35.c | 14 +-- > hw/ppc/spapr.c | 18 +--- > hw/ppc/spapr_drc.c | 3 +- > include/qom/object.h | 48 ++++++++-- > memory.c | 15 +-- > qom/object.c | 212 ++++++++++++++++++++++++++++++++++++++----- > target/arm/cpu.c | 22 +---- > target/i386/sev.c | 106 ++-------------------- > ui/console.c | 4 +- > 14 files changed, 282 insertions(+), 318 deletions(-) Series: Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Paolo, would you queue it? > > -- > 2.20.1 > > Changelog: > v1->v2: > - Update sci_int directly instead of using stack variable > - Defining an enhanced ObjectPropertyFlags instead of just 'readonly' > - Erroring out directly (instead of using gotos) on default setters > - Retaining lack of errp passing when it wasn't there > v2->v3: > - Rename flags _RD to _READ and _WR to _WRITE > - Add a convenience _READWRITE flag > - Drop the usage of UL in the bit flag definitions > v3->v4: > - Drop changes to hw/vfio/pci-quirks.c > v4->v5: > - Rebase on latest master > - Available here: https://github.com/franciozzy/qemu/tree/autosetters > v5->v6: > - Fix build error introduced in rebase
Hi Marc/Paolo, > On Feb 4, 2020, at 1:41 PM, Marc-André Lureau <marcandre.lureau@gmail.com> wrote: > > Hi > > On Tue, Feb 4, 2020 at 2:16 PM Felipe Franciosi <felipe@nutanix.com> wrote: >> >> This improves the family of object_property_add_uintXX_ptr helpers by enabling >> a default getter/setter only when desired. To prevent an API behavioural change >> (from clients that already used these helpers and did not want a setter), we >> add a OBJ_PROP_FLAG_READ flag that allow clients to only have a getter. Patch 1 >> enhances the API and modify current users. >> >> While modifying the clients of the API, a couple of improvement opportunities >> were observed in ich9. These were added in separate patches (2 and 3). >> >> Patch 4 cleans up a lot of existing code by moving various objects to the >> enhanced API. Previously, those objects had their own getters/setters that only >> updated the values without further checks. Some of them actually lacked a check >> for setting overflows, which could have resulted in undesired values being set. >> The new default setters include a check for that, not updating the values in >> case of errors (and propagating them). If they did not provide an error >> pointer, then that behaviour was maintained. >> >> Felipe Franciosi (4): >> qom/object: enable setter for uint types >> ich9: fix getter type for sci_int property >> ich9: Simplify ich9_lpc_initfn >> qom/object: Use common get/set uint helpers >> >> hw/acpi/ich9.c | 99 ++------------------ >> hw/acpi/pcihp.c | 7 +- >> hw/acpi/piix4.c | 12 +-- >> hw/isa/lpc_ich9.c | 27 ++---- >> hw/misc/edu.c | 13 +-- >> hw/pci-host/q35.c | 14 +-- >> hw/ppc/spapr.c | 18 +--- >> hw/ppc/spapr_drc.c | 3 +- >> include/qom/object.h | 48 ++++++++-- >> memory.c | 15 +-- >> qom/object.c | 212 ++++++++++++++++++++++++++++++++++++++----- >> target/arm/cpu.c | 22 +---- >> target/i386/sev.c | 106 ++-------------------- >> ui/console.c | 4 +- >> 14 files changed, 282 insertions(+), 318 deletions(-) > > Series: > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > Paolo, would you queue it? Ping: did this get lost? F. > >> >> -- >> 2.20.1 >> >> Changelog: >> v1->v2: >> - Update sci_int directly instead of using stack variable >> - Defining an enhanced ObjectPropertyFlags instead of just 'readonly' >> - Erroring out directly (instead of using gotos) on default setters >> - Retaining lack of errp passing when it wasn't there >> v2->v3: >> - Rename flags _RD to _READ and _WR to _WRITE >> - Add a convenience _READWRITE flag >> - Drop the usage of UL in the bit flag definitions >> v3->v4: >> - Drop changes to hw/vfio/pci-quirks.c >> v4->v5: >> - Rebase on latest master >> - Available here: https://github.com/franciozzy/qemu/tree/autosetters >> v5->v6: >> - Fix build error introduced in rebase > > > > -- > Marc-André Lureau
On 03/03/20 09:05, Felipe Franciosi wrote: >> >> Paolo, would you queue it? > Ping: did this get lost? Yes, I have queued it now and fixed the conflicts. Paolo
> On Mar 3, 2020, at 8:25 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 03/03/20 09:05, Felipe Franciosi wrote: >>> >>> Paolo, would you queue it? >> Ping: did this get lost? > > Yes, I have queued it now and fixed the conflicts. > > Paolo > Thanks! Sorry for conflicts, I should have pinged sooner. F.