Re: [Qemu-devel] [PATCH 2.1 09/28] vl: redo -object parsing

[Paolo Bonzini]

Il 07/03/2014 03:56, Hu Tao ha scritto:
> > -    obj = object_new(type);
> > -    if (qemu_opt_foreach(opts, object_set_property, obj, 1) < 0) {
> > -        object_unref(obj);
> > -        return -1;
> > +    qdict_del(pdict, "qom-type");
> > +    visit_type_str(opts_get_visitor(ov), &type, "qom-type", &err);
> > +    if (err) {
> > +        goto out;
> >      }
>
> Can be moved up right before creating qdict.
>
> > -    if (!object_dynamic_cast(obj, TYPE_USER_CREATABLE)) {
> > -        error_setg(&local_err, "object '%s' isn't supported by -object",
> > -                   id);
> > +    qdict_del(pdict, "id");
> > +    visit_type_str(opts_get_visitor(ov), &id, "id", &err);
> > +    if (err) {
> >          goto out;
> >      }
>
> Can be moved up right before creating qdict.

In both cases I prefer to keep the qdict_del and visit_type_str together.

> > -    user_creatable_complete(obj, &local_err);
> > -    if (local_err) {
> > +    object_add(type, id, pdict, opts_get_visitor(ov), &err);
>
> I think it's better to move object_add() from qmp.c to qom/object.c.

No, I don't think so.  qom/object.c is not using QDict.  It is common 
for "human user interface" files (hmp.c, ui/gtk.c, in this case vl.c) to 
use qmp.c, the structure should be

          hmp.c, ui/gtk.c, vl.c
                 |
               qmp.c
                 |
        qom/, cpus.c, etc.

We could move parts of qmp.c to qom/qmp.c, that would be fine.

Paolo

> > +    if (err) {
> >          goto out;
> >      }
> > -
> > -    object_property_add_child(container_get(object_get_root(), "/objects"),
> > -                              id, obj, &local_err);
> > +    visit_end_struct(opts_get_visitor(ov), &err);
> > +    if (err) {
> > +        qmp_object_del(id, NULL);
> > +    }
> >
> >  out:
> > -    object_unref(obj);
> > -    if (local_err) {
> > -        qerror_report_err(local_err);
> > -        error_free(local_err);
> > -        return -1;
> > +    opts_visitor_cleanup(ov);
> > +
> > +    QDECREF(pdict);
> > +    g_free(id);
> > +    g_free(type);
> > +    g_free(dummy);
> > +    if (err) {
> > +        qerror_report_err(err);
> >      }
> >      return 0;
> >  }
> > --
> > 1.8.5.3