On 2026/09/04 11:58 PM, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <[email protected]>
The patch itself looks correct, but the commit message is missing a body. At
minimum, it should describe what is being converted and why.
One thing worth calling out explicitly is `SPAPR_CAP_HPT_MAXPAGESIZE`. This is
not a purely mechanical conversion: the old property type string was `"int"`,
while `spapr_cap_get_pagesize()` and `spapr_cap_set_pagesize()` use
`visit_type_size()`. Thus, the property has effectively had size semantics
despite the old `"int"` type string.
Switching this to `&size_type_info` therefore also fixes a pre-existing mismatch
between the property's declared type and the visitor used by its getter/setter.
This seems worth documenting in the commit message so that future readers
understand that the change is intentional.
The conversion itself, including the switch to
`object_class_property_add_qapi()` in `spapr_caps_add_properties()`, looks
correct to me.
Thanks,
Amit
> ---
> hw/ppc/spapr_caps.c | 39 ++++++++++++++++++++-------------------
> 1 file changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 0d659e9b0251..e622c79eb52a 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -24,6 +24,7 @@
>
> #include "qemu/osdep.h"
> #include "qemu/error-report.h"
> +#include "qapi/qapi-builtin-type-infos.h"
> #include "qapi/error.h"
> #include "qapi/visitor.h"
> #include "system/hw_accel.h"
> @@ -59,7 +60,7 @@ typedef struct SpaprCapabilityInfo {
> /* Getter and Setter Function Pointers */
> ObjectPropertyAccessor *get;
> ObjectPropertyAccessor *set;
> - const char *type;
> + const QAPITypeInfo *type;
> /* Possible values if this is a custom string type */
> SpaprCapPossible *possible;
> /* Make sure the virtual hardware can support this capability */
> @@ -725,7 +726,7 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> .index = SPAPR_CAP_HTM,
> .get = spapr_cap_get_bool,
> .set = spapr_cap_set_bool,
> - .type = "bool",
> + .type = &bool_type_info,
> .apply = cap_htm_apply,
> },
> [SPAPR_CAP_VSX] = {
> @@ -734,7 +735,7 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> .index = SPAPR_CAP_VSX,
> .get = spapr_cap_get_bool,
> .set = spapr_cap_set_bool,
> - .type = "bool",
> + .type = &bool_type_info,
> .apply = cap_vsx_apply,
> },
> [SPAPR_CAP_DFP] = {
> @@ -743,7 +744,7 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> .index = SPAPR_CAP_DFP,
> .get = spapr_cap_get_bool,
> .set = spapr_cap_set_bool,
> - .type = "bool",
> + .type = &bool_type_info,
> .apply = cap_dfp_apply,
> },
> [SPAPR_CAP_CFPC] = {
> @@ -752,7 +753,7 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> .index = SPAPR_CAP_CFPC,
> .get = spapr_cap_get_string,
> .set = spapr_cap_set_string,
> - .type = "string",
> + .type = &str_type_info,
> .possible = &cap_cfpc_possible,
> .apply = cap_safe_cache_apply,
> },
> @@ -762,7 +763,7 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> .index = SPAPR_CAP_SBBC,
> .get = spapr_cap_get_string,
> .set = spapr_cap_set_string,
> - .type = "string",
> + .type = &str_type_info,
> .possible = &cap_sbbc_possible,
> .apply = cap_safe_bounds_check_apply,
> },
> @@ -774,7 +775,7 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> .index = SPAPR_CAP_IBS,
> .get = spapr_cap_get_string,
> .set = spapr_cap_set_string,
> - .type = "string",
> + .type = &str_type_info,
> .possible = &cap_ibs_possible,
> .apply = cap_safe_indirect_branch_apply,
> },
> @@ -784,7 +785,7 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> .index = SPAPR_CAP_HPT_MAXPAGESIZE,
> .get = spapr_cap_get_pagesize,
> .set = spapr_cap_set_pagesize,
> - .type = "int",
> + .type = &size_type_info,
> .apply = cap_hpt_maxpagesize_apply,
> .cpu_apply = cap_hpt_maxpagesize_cpu_apply,
> },
> @@ -794,7 +795,7 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> .index = SPAPR_CAP_NESTED_KVM_HV,
> .get = spapr_cap_get_bool,
> .set = spapr_cap_set_bool,
> - .type = "bool",
> + .type = &bool_type_info,
> .apply = cap_nested_kvm_hv_apply,
> },
> [SPAPR_CAP_NESTED_PAPR] = {
> @@ -803,7 +804,7 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> .index = SPAPR_CAP_NESTED_PAPR,
> .get = spapr_cap_get_bool,
> .set = spapr_cap_set_bool,
> - .type = "bool",
> + .type = &bool_type_info,
> .apply = cap_nested_papr_apply,
> },
> [SPAPR_CAP_LARGE_DECREMENTER] = {
> @@ -812,7 +813,7 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> .index = SPAPR_CAP_LARGE_DECREMENTER,
> .get = spapr_cap_get_bool,
> .set = spapr_cap_set_bool,
> - .type = "bool",
> + .type = &bool_type_info,
> .apply = cap_large_decr_apply,
> .cpu_apply = cap_large_decr_cpu_apply,
> },
> @@ -822,7 +823,7 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> .index = SPAPR_CAP_CCF_ASSIST,
> .get = spapr_cap_get_bool,
> .set = spapr_cap_set_bool,
> - .type = "bool",
> + .type = &bool_type_info,
> .apply = cap_ccf_assist_apply,
> },
> [SPAPR_CAP_FWNMI] = {
> @@ -831,7 +832,7 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> .index = SPAPR_CAP_FWNMI,
> .get = spapr_cap_get_bool,
> .set = spapr_cap_set_bool,
> - .type = "bool",
> + .type = &bool_type_info,
> .apply = cap_fwnmi_apply,
> },
> [SPAPR_CAP_RPT_INVALIDATE] = {
> @@ -840,7 +841,7 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> .index = SPAPR_CAP_RPT_INVALIDATE,
> .get = spapr_cap_get_bool,
> .set = spapr_cap_set_bool,
> - .type = "bool",
> + .type = &bool_type_info,
> .apply = cap_rpt_invalidate_apply,
> },
> [SPAPR_CAP_AIL_MODE_3] = {
> @@ -849,7 +850,7 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> .index = SPAPR_CAP_AIL_MODE_3,
> .get = spapr_cap_get_bool,
> .set = spapr_cap_set_bool,
> - .type = "bool",
> + .type = &bool_type_info,
> .apply = cap_ail_mode_3_apply,
> },
> [SPAPR_CAP_DAWR1] = {
> @@ -858,7 +859,7 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> .index = SPAPR_CAP_DAWR1,
> .get = spapr_cap_get_bool,
> .set = spapr_cap_set_bool,
> - .type = "bool",
> + .type = &bool_type_info,
> .apply = cap_dawr1_apply,
> },
> };
> @@ -1069,9 +1070,9 @@ void spapr_caps_add_properties(SpaprMachineClass *smc)
> g_autofree char *name = g_strdup_printf("cap-%s", cap->name);
> g_autofree char *desc = g_strdup(cap->description);
>
> - object_class_property_add(klass, name, cap->type,
> - cap->get, cap->set,
> - NULL, cap);
> + object_class_property_add_qapi(klass, name, cap->type,
> + cap->get, cap->set,
> + NULL, cap);
>
> object_class_property_set_description(klass, name, desc);
> }
>
> --
> 2.55.0.543.g5ebe2ebe4ea8
>
>
