[PATCH v6 0/4] Improve default object property_add uint helpers

Felipe Franciosi posted 4 patches 4 years, 2 months ago
Test docker-quick@centos7 passed
Test FreeBSD passed
Test docker-mingw@fedora passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200204131601.125677-1-felipe@nutanix.com
Maintainers: Peter Maydell <peter.maydell@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <rth@twiddle.net>, Aurelien Jarno <aurelien@aurel32.net>, Gerd Hoffmann <kraxel@redhat.com>, Jiri Slaby <jslaby@suse.cz>, Eduardo Habkost <ehabkost@redhat.com>, Aleksandar Markovic <amarkovic@wavecomp.com>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, David Gibson <david@gibson.dropbear.id.au>, "Daniel P. Berrangé" <berrange@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>
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(-)
[PATCH v6 0/4] Improve default object property_add uint helpers
Posted by Felipe Franciosi 4 years, 2 months ago
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(-)

-- 
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

Re: [PATCH v6 0/4] Improve default object property_add uint helpers
Posted by Marc-André Lureau 4 years, 2 months ago
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



-- 
Marc-André Lureau

Re: [PATCH v6 0/4] Improve default object property_add uint helpers
Posted by Felipe Franciosi 4 years, 1 month ago
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

Re: [PATCH v6 0/4] Improve default object property_add uint helpers
Posted by Paolo Bonzini 4 years, 1 month ago
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


Re: [PATCH v6 0/4] Improve default object property_add uint helpers
Posted by Felipe Franciosi 4 years, 1 month ago
> 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.