From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:58755)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1WLpNh-0007O8-MC
	for qemu-devel@nongnu.org; Fri, 07 Mar 2014 02:39:54 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1WLpNZ-0002pG-99
	for qemu-devel@nongnu.org; Fri, 07 Mar 2014 02:39:45 -0500
Received: from mail-ee0-x235.google.com ([2a00:1450:4013:c00::235]:51958)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1WLpNZ-0002pA-1M
	for qemu-devel@nongnu.org; Fri, 07 Mar 2014 02:39:37 -0500
Received: by mail-ee0-f53.google.com with SMTP id e51so1566163eek.12
	for <qemu-devel@nongnu.org>; Thu, 06 Mar 2014 23:39:36 -0800 (PST)
Sender: Paolo Bonzini <paolo.bonzini@gmail.com>
Message-ID: <531977B4.7010302@redhat.com>
Date: Fri, 07 Mar 2014 08:39:32 +0100
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1393941656-29068-1-git-send-email-pbonzini@redhat.com>	<1393941656-29068-10-git-send-email-pbonzini@redhat.com>
	<20140307025653.GD16105@G08FNSTD100614.fnst.cn.fujitsu.com>
In-Reply-To: <20140307025653.GD16105@G08FNSTD100614.fnst.cn.fujitsu.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 2.1 09/28] vl: redo -object parsing
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Hu Tao <hutao@cn.fujitsu.com>
Cc: ehabkost@redhat.com, mtosatti@redhat.com, qemu-devel@nongnu.org, a.motakis@virtualopensystems.com, imammedo@redhat.com, gaowanlong@cn.fujitsu.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
>>
>
>