Re: [Qemu-devel] [PATCH 7/8] qom: distinguish "legacy" property type name from QOM type name

[Anthony Liguori]

On 12/16/2011 06:01 AM, Paolo Bonzini wrote:
> For non-string properties, there is no reason to distinguish type names
> such as "uint32" or "hex32".  Restrict those to legacy properties.
>
> Signed-off-by: Paolo Bonzini<address@hidden>
> ---
>   hw/qdev-properties.c |   12 ++++++++----
>   hw/qdev.c            |    9 ++++++---
>   hw/qdev.h            |    1 +
>   3 files changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index 5e8dd9a..6b6732e 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -86,7 +86,8 @@ static void set_bit(DeviceState *dev, Visitor *v, void 
> *opaque,
>   }
>
>   PropertyInfo qdev_prop_bit = {
> -    .name  = "on/off",
> +    .name  = "boolean",
> +    .legacy_name  = "on/off",
>       .type  = PROP_TYPE_BIT,
>       .size  = sizeof(uint32_t),
>       .parse = parse_bit,
> @@ -189,7 +190,8 @@ static int print_hex8(DeviceState *dev, Property *prop, 
> char *dest, size_t len)
>   }
>
>   PropertyInfo qdev_prop_hex8 = {
> -    .name  = "hex8",
> +    .name  = "uint8",
> +    .legacy_name  = "hex8",
>       .type  = PROP_TYPE_UINT8,
>       .size  = sizeof(uint8_t),
>       .parse = parse_hex8,
> @@ -397,7 +399,8 @@ static int print_hex32(DeviceState *dev, Property *prop, 
> char *dest, size_t len)
>   }
>
>   PropertyInfo qdev_prop_hex32 = {
> -    .name  = "hex32",
> +    .name  = "uint32",
> +    .legacy_name  = "hex32",
>       .type  = PROP_TYPE_UINT32,
>       .size  = sizeof(uint32_t),
>       .parse = parse_hex32,
> @@ -485,7 +488,8 @@ static int print_hex64(DeviceState *dev, Property *prop, 
> char *dest, size_t len)
>   }
>
>   PropertyInfo qdev_prop_hex64 = {
> -    .name  = "hex64",
> +    .name  = "uint64",
> +    .legacy_name  = "hex64",
>       .type  = PROP_TYPE_UINT64,
>       .size  = sizeof(uint64_t),
>       .parse = parse_hex64,
> diff --git a/hw/qdev.c b/hw/qdev.c
> index c020a6f..d76861e 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -218,13 +218,15 @@ int qdev_device_help(QemuOpts *opts)
>           if (!prop->info->parse) {
>               continue;           /* no way to set it, don't show */
>           }
> -        error_printf("%s.%s=%s\n", info->name, prop->name, prop->info->name);
> +        error_printf("%s.%s=%s\n", info->name, prop->name,
> +                     prop->info->legacy_name ?: prop->info->name);
>       }
>       for (prop = info->bus_info->props; prop&&  prop->name; prop++) {
>           if (!prop->info->parse) {
>               continue;           /* no way to set it, don't show */
>           }
> -        error_printf("%s.%s=%s\n", info->name, prop->name, prop->info->name);
> +        error_printf("%s.%s=%s\n", info->name, prop->name,
> +                     prop->info->legacy_name ?: prop->info->name);
>       }
>       return 1;
>   }
> @@ -1183,7 +1185,8 @@ void qdev_property_add_legacy(DeviceState *dev, Property 
> *prop,
>   {
>       gchar *type;
>
> -    type = g_strdup_printf("legacy<%s>", prop->info->name);
> +    type = g_strdup_printf("legacy<%s>",
> +                           prop->info->legacy_name ?: prop->info->name);

I think this confuses the legacy type with the legacy property names.

I think it would be better to do 'legacy-%s' as then it's at least clear when 
something is a type name vs. a property name.

Regards,

Anthony Liguori

>       qdev_property_add(dev, prop->name, type,
>                         prop->info->print ? qdev_get_legacy_property : NULL,
> diff --git a/hw/qdev.h b/hw/qdev.h
> index 9778123..c7d9535 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -156,6 +156,7 @@ enum PropertyType {
>
>   struct PropertyInfo {
>       const char *name;
> +    const char *legacy_name;
>       size_t size;
>       enum PropertyType type;
>       int64_t min;