From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47684) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aAqgq-0003Qu-Vl for qemu-devel@nongnu.org; Sun, 20 Dec 2015 21:59:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aAqgn-0002ix-NK for qemu-devel@nongnu.org; Sun, 20 Dec 2015 21:59:12 -0500 Received: from [59.151.112.132] (port=19683 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aAqgn-0002iC-2W for qemu-devel@nongnu.org; Sun, 20 Dec 2015 21:59:09 -0500 References: <1450436632-23980-1-git-send-email-caoj.fnst@cn.fujitsu.com> <1450436632-23980-4-git-send-email-caoj.fnst@cn.fujitsu.com> <56768170.2080602@gmail.com> <56768766.1040300@cn.fujitsu.com> <56768F2B.5030104@redhat.com> From: Cao jin Message-ID: <56776B07.7000102@cn.fujitsu.com> Date: Mon, 21 Dec 2015 10:59:19 +0800 MIME-Version: 1.0 In-Reply-To: <56768F2B.5030104@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/5] PXB: convert to realize() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcel Apfelbaum , qemu-devel@nongnu.org Cc: ehabkost@redhat.com, mst@redhat.com, Markus Armbruster , pbonzini@redhat.com, leon.alrae@imgtec.com, =?UTF-8?Q?Andreas_F=c3=a4rber?= , aurelien@aurel32.net, rth@twiddle.net On 12/20/2015 07:21 PM, Marcel Apfelbaum wrote: > On 12/20/2015 12:48 PM, Cao jin wrote: >> Hi, >> >> On 12/20/2015 06:22 PM, Marcel Apfelbaum wrote: [...] >>>> + >>>> +err_register_bus: >>>> + object_unref(OBJECT(ds)); >>>> + object_unref(OBJECT(bds)); >>>> + object_unref(OBJECT(bus)); >>> >>> >>> The order should be in the reverse order of creation: >>> bds, bus, ds >>> >> >> Ok, I can do that. But it seems the order here doesn`t matter? Is >> there dependency among these three? > > Yes, there is a dependency: > At first the pxb host (ds) is created, then the bus (bus) is created as > host's child (see pci_bus_new) > and in the end a pci bridge (bds) is attached to the bus (see qdev_create). > Yup...thanks for reminding, I did read the code trying to find the parent relationship...but seem I didn`t read it thoroughly:-[ > By the way, indeed you should call object_unparent at least for the > pxb_host(ds), but you may want to > check the others parent relationship as well. > yes, but I think you are saying: object_unparent(bus), right? the relationship seems is: pxb host-->(child property)bus-->(link property)bds Another question: because some of this series is CCed to qemu-trivial(which means: reviewed-by?) by other maintainer, so next time, do I need to send the whole series with "v2", or the rest? >> >>> >>>> } >>>> >>>> static void pxb_dev_exitfn(PCIDevice *pci_dev) >>>> @@ -259,7 +263,7 @@ static void pxb_dev_class_init(ObjectClass *klass, >>>> void *data) >>>> DeviceClass *dc = DEVICE_CLASS(klass); >>>> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); >>>> >>>> - k->init = pxb_dev_initfn; >>>> + k->realize = pxb_dev_realize; >>>> k->exit = pxb_dev_exitfn; >>> >>> If init is converted to realize, maybe the exit should be converted to >>> unrealize? >>> >> >> Yup, I agree with you from the point that the names should be antonym. >> But it seems there is no PCIDeviceClass.unrealize:( > > You are right. The pci_qdev_unrealize ultimately calls exit. But the > same goes for init, pci_qdev_realize calls for pc->realize. > This is the reason I chose to use init/exit instead of the strange > realize/exit. > > But since the intention is to get rid of init, I am not against it. > >> >> And I am also not aware why there is no comment for .exit while there >> is for .init. It is appreciated if somebody could tell me the history >> O:-) > > I'll add Markus, Andreas and Michael (the PCI maintainer), maybe they > have a better insight to this. > > On the other hand you should continue with the patch and leave the > "unrealize" until we'll know more :) Got it;) > > Thanks, > Marcel > >> >>> >>> Thanks, >>> Marcel >>> >>>> k->vendor_id = PCI_VENDOR_ID_REDHAT; >>>> k->device_id = PCI_DEVICE_ID_REDHAT_PXB; >>>> >>> >>> >>> >>> . >>> >> > > > > . > -- Yours Sincerely, Cao Jin