From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53503) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZfMg1-0000kf-32 for qemu-devel@nongnu.org; Fri, 25 Sep 2015 02:40:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZfMfx-0001By-3H for qemu-devel@nongnu.org; Fri, 25 Sep 2015 02:40:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46606) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZfMfw-0001Bd-VO for qemu-devel@nongnu.org; Fri, 25 Sep 2015 02:40:09 -0400 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> <5603B5C8.7080201@cn.fujitsu.com> From: Jason Wang Message-ID: <5604EC48.8070804@redhat.com> Date: Fri, 25 Sep 2015 14:40:08 +0800 MIME-Version: 1.0 In-Reply-To: <5603B5C8.7080201@cn.fujitsu.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v11 01/12] qmp: delete qemu opts when delete an object List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Yang Hongyang , Markus Armbruster Cc: thuth@redhat.com, zhang.zhanghailiang@huawei.com, lizhijian@cn.fujitsu.com, qemu-devel@nongnu.org, stefanha@redhat.com, Paolo Bonzini On 09/24/2015 04:35 PM, Yang Hongyang wrote: > 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. Will drop this patch from my tree. Thanks > >> >> 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) >> . >> >