Re: [Qemu-devel] [PATCH 3/5] PXB: convert to realize()

[Cao jin <caoj.fnst@cn.fujitsu.com>]

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