Re: [Qemu-devel] [PATCH] qmp: show QOM properties in device-list-properties

[Paolo Bonzini <pbonzini@redhat.com>]

Il 20/05/2014 14:29, Stefan Hajnoczi ha scritto:
> Devices can use a mix of qdev and QOM properties.  Currently only the
> qdev properties are displayed by device-list-properties.
>
> This patch extends the property enumeration algorithm to also display
> QOM properties (excluding the implicit "type", "realized",
> "hotpluggable", and "parent_bus" properties).
>
> When a qdev property exists, use the qdev type name to preserve
> backwards compatibility.  QOM type names can be different for bool (qdev
> on/off) and str (used by qdev pointers).
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> Here is a demo:
>
> $ cat test.py
> #!/usr/bin/env python
> import os
> import sys; sys.path.append(os.path.join(os.path.dirname(__file__), 'tests', 'qemu-iotests'))
> import iotests
>
> iotests.qemu_args = ['x86_64-softmmu/qemu-system-x86_64']
>
> vm = iotests.VM()
> vm.launch()
> print vm.qmp('device-list-properties', typename='e1000')
> print vm.qmp('device-list-properties', typename='virtio-blk-pci')
> print vm.qmp('device-list-properties', typename='virtio-scsi-pci')
> vm.shutdown()
>
> $ ./test.py
> {u'return': [{u'type': u'on/off', u'name': u'command_serr_enable'}, {u'type': u'on/off', u'name': u'multifunction'}, {u'type': u'uint32', u'name': u'rombar'}, {u'type': u'str', u'name': u'romfile'}, {u'type': u'pci-devfn', u'name': u'addr'}, {u'type': u'on/off', u'name': u'mitigation'}, {u'type': u'on/off', u'name': u'autonegotiation'}, {u'type': u'int32', u'name': u'bootindex'}, {u'type': u'netdev', u'name': u'netdev'}, {u'type': u'vlan', u'name': u'vlan'}, {u'type': u'macaddr', u'name': u'mac'}]}
> {u'return': [{u'type': u'child<virtio-blk-device>', u'name': u'virtio-backend'}, {u'type': u'on/off', u'name': u'command_serr_enable'}, {u'type': u'on/off', u'name': u'multifunction'}, {u'type': u'uint32', u'name': u'rombar'}, {u'type': u'str', u'name': u'romfile'}, {u'type': u'pci-devfn', u'name': u'addr'}, {u'type': u'iothread', u'name': u'x-iothread'}, {u'type': u'on/off', u'name': u'scsi'}, {u'type': u'on/off', u'name': u'config-wce'}, {u'type': u'str', u'name': u'serial'}, {u'type': u'uint32', u'name': u'secs'}, {u'type': u'uint32', u'name': u'heads'}, {u'type': u'uint32', u'name': u'cyls'}, {u'type': u'uint32', u'name': u'discard_granularity'}, {u'type': u'int32', u'name': u'bootindex'}, {u'type': u'uint32', u'name': u'opt_io_size'}, {u'type': u'uint16', u'name': u'min_io_size'}, {u'
>  type': u'blocksize', u'name': u'physical_block_size'}, {u'type': u'blocksize', u'name': u'logical_block_size'}, {u'type': u'drive', u'name': u'drive'}, {u'type': u'on/off', u'name': u'event!
>  _idx'}, {u'type': u'on/off', u'name': u'indirect_desc'}, {u'type': u'on/off', u'name': u'x-data-plane'}, {u'type': u'uint32', u'name': u'vectors'}, {u'type': u'on/off', u'name': u'ioeventfd'}, {u'type': u'uint32', u'name': u'class'}]}
> {u'return': [{u'type': u'child<virtio-scsi-device>', u'name': u'virtio-backend'}, {u'type': u'on/off', u'name': u'command_serr_enable'}, {u'type': u'on/off', u'name': u'multifunction'}, {u'type': u'uint32', u'name': u'rombar'}, {u'type': u'str', u'name': u'romfile'}, {u'type': u'pci-devfn', u'name': u'addr'}, {u'type': u'uint32', u'name': u'cmd_per_lun'}, {u'type': u'uint32', u'name': u'max_sectors'}, {u'type': u'uint32', u'name': u'num_queues'}, {u'type': u'on/off', u'name': u'param_change'}, {u'type': u'on/off', u'name': u'hotplug'}, {u'type': u'on/off', u'name': u'event_idx'}, {u'type': u'on/off', u'name': u'indirect_desc'}, {u'type': u'uint32', u'name': u'vectors'}, {u'type': u'on/off', u'name': u'ioeventfd'}]}
>
>  qmp.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 76 insertions(+), 23 deletions(-)
>
> diff --git a/qmp.c b/qmp.c
> index a7f432b..cb2577f 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -431,11 +431,57 @@ ObjectTypeInfoList *qmp_qom_list_types(bool has_implements,
>      return ret;
>  }
>
> +/* Return a DevicePropertyInfo for a qdev property.
> + *
> + * If a qdev property with the given name does not exist, use the given default
> + * type.  If the qdev property info should not be shown, return NULL.
> + *
> + * The caller must free the return value.
> + */
> +static DevicePropertyInfo *make_device_property_info(ObjectClass *klass,
> +                                                     const char *name,
> +                                                     const char *default_type)
> +{
> +    DevicePropertyInfo *info;
> +    Property *prop;
> +
> +    do {
> +        for (prop = DEVICE_CLASS(klass)->props; prop && prop->name; prop++) {
> +            if (strcmp(name, prop->name) != 0) {
> +                continue;
> +            }
> +
> +            /*
> +             * TODO Properties without a parser are just for dirty hacks.
> +             * qdev_prop_ptr is the only such PropertyInfo.  It's marked
> +             * for removal.  This conditional should be removed along with
> +             * it.
> +             */
> +            if (!prop->info->set) {
> +                return NULL;           /* no way to set it, don't show */
> +            }
> +
> +            info = g_malloc0(sizeof(*info));
> +            info->name = g_strdup(prop->name);
> +            info->type = g_strdup(prop->info->legacy_name ?: prop->info->name);
> +            return info;
> +        }
> +        klass = object_class_get_parent(klass);
> +    } while (klass != object_class_by_name(TYPE_DEVICE));
> +
> +    /* Not a qdev property, use the default type */
> +    info = g_malloc0(sizeof(*info));
> +    info->name = g_strdup(name);
> +    info->type = g_strdup(default_type);
> +    return info;
> +}
> +
>  DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
>                                                     Error **errp)
>  {
>      ObjectClass *klass;
> -    Property *prop;
> +    Object *obj;
> +    ObjectProperty *prop;
>      DevicePropertyInfoList *prop_list = NULL;
>
>      klass = object_class_by_name(typename);
> @@ -451,32 +497,39 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
>          return NULL;
>      }
>
> -    do {
> -        for (prop = DEVICE_CLASS(klass)->props; prop && prop->name; prop++) {
> -            DevicePropertyInfoList *entry;
> -            DevicePropertyInfo *info;
> +    obj = object_new(typename);
>
> -            /*
> -             * TODO Properties without a parser are just for dirty hacks.
> -             * qdev_prop_ptr is the only such PropertyInfo.  It's marked
> -             * for removal.  This conditional should be removed along with
> -             * it.
> -             */
> -            if (!prop->info->set) {
> -                continue;           /* no way to set it, don't show */
> -            }
> +    QTAILQ_FOREACH(prop, &obj->properties, node) {
> +        DevicePropertyInfo *info;
> +        DevicePropertyInfoList *entry;
> +
> +        /* Skip Object and DeviceState properties */
> +        if (strcmp(prop->name, "type") == 0 ||
> +            strcmp(prop->name, "realized") == 0 ||
> +            strcmp(prop->name, "hotpluggable") == 0 ||
> +            strcmp(prop->name, "parent_bus") == 0) {
> +            continue;
> +        }
>
> -            info = g_malloc0(sizeof(*info));
> -            info->name = g_strdup(prop->name);
> -            info->type = g_strdup(prop->info->legacy_name ?: prop->info->name);
> +        /* Skip legacy properties since they are just string versions of
> +         * properties that we already list.
> +         */
> +        if (strstart(prop->name, "legacy-", NULL)) {
> +            continue;
> +        }
>
> -            entry = g_malloc0(sizeof(*entry));
> -            entry->value = info;
> -            entry->next = prop_list;
> -            prop_list = entry;
> +        info = make_device_property_info(klass, prop->name, prop->type);
> +        if (!info) {
> +            continue;
>          }
> -        klass = object_class_get_parent(klass);
> -    } while (klass != object_class_by_name(TYPE_DEVICE));
> +
> +        entry = g_malloc0(sizeof(*entry));
> +        entry->value = info;
> +        entry->next = prop_list;
> +        prop_list = entry;
> +    }
> +
> +    object_unref(obj);
>
>      return prop_list;
>  }
>

Stefan, was this never applied?

Paolo