(CCing libvirt people, as I forgot to CC them) On Mon, Oct 31, 2016 at 03:07:23PM +0100, Igor Mammedov wrote:
On Fri, 28 Oct 2016 23:48:06 -0200 Eduardo Habkost <ehabkost@redhat.com> wrote:
When an abstract class is used on device-list-properties, we can simply return the class properties registered for the class.
This will be useful if management software needs to query for supported options that apply to all devices of a given type (e.g. options supported by all CPU models, options supported by all PCI devices). Patch looks fine to me but I'm not qmp interface guru so I'd leave review up to maintainers.
One question though, How would management software discover typename of abstract class?
It depends on the use case. On some cases, management may already have bus-specific logic that will know what's the base type it needs to query (e.g. it may query "pci-device" to find out if all PCI devices support a given option). On other cases, it may be discovered using other commands. For the CPU case, I will propose adding the base QOM CPU typename in the query-target command.
Perhaps this patch should be part of some other series.
This is a valid point. In this case, it depends on the approach we want to take: do we want to provide a flexible interface for management, let them find ways to make it useful and give us feedback on what is lacking; or do we want to provide the new interface only after we have specified the complete solution for the problem? I don't know the answer. I will let the qdev/QOM/QMP maintainers answer that.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Changes series v1 -> v2: * (none)
Changes series v2 -> v3: * Reworded commit message
Changes series v3 -> v4: * (none) --- qmp.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/qmp.c b/qmp.c index a06cb7b..1e7e60d 100644 --- a/qmp.c +++ b/qmp.c @@ -518,7 +518,7 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename, Error **errp) { ObjectClass *klass; - Object *obj; + Object *obj = NULL; ObjectProperty *prop; ObjectPropertyIterator iter; DevicePropertyInfoList *prop_list = NULL; @@ -537,19 +537,16 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename, }
if (object_class_is_abstract(klass)) { - error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "name", - "non-abstract device type"); - return NULL; - } - - if (DEVICE_CLASS(klass)->cannot_destroy_with_object_finalize_yet) { - error_setg(errp, "Can't list properties of device '%s'", typename); - return NULL; + object_class_property_iter_init(&iter, klass); + } else { + if (DEVICE_CLASS(klass)->cannot_destroy_with_object_finalize_yet) { + error_setg(errp, "Can't list properties of device '%s'", typename); + return NULL; + } + obj = object_new(typename); + object_property_iter_init(&iter, obj); }
- obj = object_new(typename); - - object_property_iter_init(&iter, obj); while ((prop = object_property_iter_next(&iter))) { DevicePropertyInfo *info; DevicePropertyInfoList *entry;
-- Eduardo