From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:54631)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <yanghy@cn.fujitsu.com>) id 1Zf20F-0004Zm-5M
	for qemu-devel@nongnu.org; Thu, 24 Sep 2015 04:35:44 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <yanghy@cn.fujitsu.com>) id 1Zf20B-0002ZB-LB
	for qemu-devel@nongnu.org; Thu, 24 Sep 2015 04:35:43 -0400
Received: from [59.151.112.132] (port=43613 helo=heian.cn.fujitsu.com)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <yanghy@cn.fujitsu.com>) id 1Zf209-0002XA-Vy
	for qemu-devel@nongnu.org; Thu, 24 Sep 2015 04:35:39 -0400
Message-ID: <5603B5C8.7080201@cn.fujitsu.com>
Date: Thu, 24 Sep 2015 16:35:20 +0800
From: Yang Hongyang <yanghy@cn.fujitsu.com>
MIME-Version: 1.0
References: <1442405768-23019-1-git-send-email-yanghy@cn.fujitsu.com>	<1442405768-23019-2-git-send-email-yanghy@cn.fujitsu.com>
	<87a8sctg74.fsf@blackfin.pond.sub.org>
In-Reply-To: <87a8sctg74.fsf@blackfin.pond.sub.org>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v11 01/12] qmp: delete qemu opts when
 delete an object
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: Markus Armbruster <armbru@redhat.com>
Cc: thuth@redhat.com, zhang.zhanghailiang@huawei.com, lizhijian@cn.fujitsu.com, jasowang@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, Paolo Bonzini <pbonzini@redhat.com>

On 09/24/2015 03:43 PM, Markus Armbruster wrote:
> This has finally reached the front of my review queue.  I apologize for
> the loooong delay.
>
> Copying Paolo for another pair of eyeballs (he wrote this code).
>
[...]
>> +
>> +    opts = qemu_opts_find(qemu_find_opts_err("object", NULL), id);
>> +    qemu_opts_del(opts);
>
> qemu_find_opts_err("object", &error_abort) please, because when it
> fails, we want to die right away, not when the null pointer it returns
> gets dereferenced.

Thanks for the review.
Jason, do you want me to propose a fix on top of this series or simply drop
this for now because this patch is an independent bug fix and won't affect the
other filter patch series.

>
> Same sloppiness in netdev_del_completion() and qmp_netdev_del(), not
> your patch's fault.
>
> Elsewhere, we store the QemuOpts in the object just so we can delete it:
> DeviceState, DriveInfo.  Paolo, what do you think?

I don't get it. Currently, only objects created at the beginning through
QEMU command line will be stored in the QemuOpts, objects that created
with object_add won't stored in QemuOpts. Do you mean for DeviceState,
DriveInfo they store there QemuOpts explicity so that they can delete it?
Why don't we just delete it from objects directly instead?

>
>>   }
>>
>>   MemoryDeviceInfoList *qmp_query_memory_devices(Error **errp)
> .
>

-- 
Thanks,
Yang.