Re: [Qemu-devel] [PATCH 3/3] fix qmp/hmp query-memdev not repporting IDs of memory backends

[Igor Mammedov]

On Tue, 3 Jan 2017 09:34:40 -0600
Eric Blake <address@hidden> wrote:

> On 01/02/2017 09:44 AM, Igor Mammedov wrote:
> 
> s/repporting/reporting/ in the subject
> 
> > Do it by adding 'id' property to hostmem backends and fetch it
> > in query-memdev from object directly.
> > 
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> 
> > +++ b/qom/object_interfaces.c
> > @@ -4,6 +4,7 @@
> >  #include "qemu/module.h"
> >  #include "qapi-visit.h"
> >  #include "qapi/opts-visitor.h"
> > +#include "qapi/qmp/qstring.h"
> >  
> >  void user_creatable_complete(Object *obj, Error **errp)
> >  {
> > @@ -35,7 +36,7 @@ bool user_creatable_can_be_deleted(UserCreatable *uc, 
> > Error **errp)
> >  }
> >  
> >  Object *user_creatable_add_type(const char *type, const char *id,
> > -                                const QDict *qdict,
> > +                                QDict *qdict,
> >                                  Visitor *v, Error **errp)
> >  {
> >      Object *obj;
> > @@ -62,6 +63,9 @@ Object *user_creatable_add_type(const char *type, const 
> > char *id,
> >  
> >      assert(qdict);
> >      obj = object_new(type);
> > +    if (object_property_find(obj, "id", NULL)) {
> > +        qdict_put(qdict, "id", qstring_from_str(id));
> > +    }
> 
> Wait. Isn't this going to inject an 'id' dict member to every use of
> user_creatable_add_type()?  But not all QAPI structs contain an id
> member.
it would 'inject' 'id' only for objects that have 'id' property.
or more exactly it would restore 'id' that is deleted earlier from
the same qdict. Look for qdict_del() usage in this file.
This way it'd be able to process both kinds of objects that implement
'id' property and ones that don't.
 
> Which means that you are now explicitly relying on the visitor
> to silently ignore garbage in the dictionary, rather than our desired
> goal of only validating if the dictionary exactly matches what the QAPI
> says it will match.
currently 'id' is mandatory, but not enforced by QAPI structs,
for all objects created with -object/object_add as it's used for
naming object in qom tree:
      object_property_add_child(object_get_objects_root(), id, obj, 
&local_err); 

> 
> I'm not sure if I like this hack, or if there is a better way to do
> things when using a strict (rather than relaxed) input visitor.
I'm not sure about a better way to do this.
Currently we use 2 visitors: opts (for cli/hmp) and qobject_input for qmp path,
which accept anything as target object could take different options.

The only things we currently enforce in code is presence of implicit 'qom-type' 
property
and 'id' property. The rest of properties are validated by object's property 
setters.

> 
> >      visit_start_struct(v, NULL, NULL, 0, &local_err);
> >      if (local_err) {
> >          goto out;
> > 
>