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

[Cao jin]

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