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

[Paolo Bonzini <pbonzini@redhat.com>]

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