[libvirt] [PATCH v4 7/8] qmp: Support abstract classes on device-list-properties

[Eduardo Habkost]

(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 at 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 at 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