From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33264) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X3M36-0004EH-AK for qemu-devel@nongnu.org; Sat, 05 Jul 2014 05:14:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X3M2z-0006LC-DS for qemu-devel@nongnu.org; Sat, 05 Jul 2014 05:14:24 -0400 Received: from mail-qa0-x22f.google.com ([2607:f8b0:400d:c00::22f]:45546) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X3M2z-0006L0-6s for qemu-devel@nongnu.org; Sat, 05 Jul 2014 05:14:17 -0400 Received: by mail-qa0-f47.google.com with SMTP id hw13so1951311qab.20 for ; Sat, 05 Jul 2014 02:14:16 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <53B7C1E3.1010103@redhat.com> Date: Sat, 05 Jul 2014 11:14:11 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1400588941-12347-1-git-send-email-stefanha@redhat.com> In-Reply-To: <1400588941-12347-1-git-send-email-stefanha@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] qmp: show QOM properties in device-list-properties List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi , qemu-devel@nongnu.org Cc: Peter Maydell , Andreas Faerber , Cole Robinson 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 > --- > 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', 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', 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