| Message ID | 1491984907-9894-1-git-send-email-bharata@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
This should be considered for ppc-for-2.10 branch of dwg's tree. On Wed, Apr 12, 2017 at 01:45:07PM +0530, Bharata B Rao wrote: > Recent commits that re-organized ICPState object missed to destroy > the object when CPU is unrealized. Fix this so that CPU unplug > doesn't abort QEMU. > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> > --- > hw/ppc/spapr_cpu_core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > index 2e689b5..4389ef4 100644 > --- a/hw/ppc/spapr_cpu_core.c > +++ b/hw/ppc/spapr_cpu_core.c > @@ -127,6 +127,7 @@ static void spapr_cpu_core_unrealizefn(DeviceState *dev, Error **errp) > PowerPCCPU *cpu = POWERPC_CPU(cs); > > spapr_cpu_destroy(cpu); > + object_unparent(cpu->intc); > cpu_remove_sync(cs); > object_unparent(obj); > } > -- > 2.7.4
On 04/12/2017 10:15 AM, Bharata B Rao wrote: > Recent commits that re-organized ICPState object missed to destroy > the object when CPU is unrealized. Fix this so that CPU unplug > doesn't abort QEMU. Indeed. > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> I am wondering if we should not be doing the unparent under spapr_cpu_destroy() or even xics_cpu_destroy(). Apart from that, Reviewed-by: Cédric Le Goater <clg@kaod.org> Thanks, C. > --- > hw/ppc/spapr_cpu_core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > index 2e689b5..4389ef4 100644 > --- a/hw/ppc/spapr_cpu_core.c > +++ b/hw/ppc/spapr_cpu_core.c > @@ -127,6 +127,7 @@ static void spapr_cpu_core_unrealizefn(DeviceState *dev, Error **errp) > PowerPCCPU *cpu = POWERPC_CPU(cs); > > spapr_cpu_destroy(cpu); > + object_unparent(cpu->intc); > cpu_remove_sync(cs); > object_unparent(obj); > } >
On Wed, Apr 12, 2017 at 10:47:39AM +0200, Cédric Le Goater wrote: > On 04/12/2017 10:15 AM, Bharata B Rao wrote: > > Recent commits that re-organized ICPState object missed to destroy > > the object when CPU is unrealized. Fix this so that CPU unplug > > doesn't abort QEMU. > > Indeed. > > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> > > I am wondering if we should not be doing the unparent under > spapr_cpu_destroy() or even xics_cpu_destroy(). Apart from > that, Thought so, but since object is created in realize routinte itself (and not in any of its callers), did the object destruction in unrealize to be symmetrical :) Regards, Bharata.
On 04/12/2017 10:53 AM, Bharata B Rao wrote: > On Wed, Apr 12, 2017 at 10:47:39AM +0200, Cédric Le Goater wrote: >> On 04/12/2017 10:15 AM, Bharata B Rao wrote: >>> Recent commits that re-organized ICPState object missed to destroy >>> the object when CPU is unrealized. Fix this so that CPU unplug >>> doesn't abort QEMU. >> >> Indeed. >> >>> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> >> >> I am wondering if we should not be doing the unparent under >> spapr_cpu_destroy() or even xics_cpu_destroy(). Apart from >> that, > > Thought so, but since object is created in realize routinte itself > (and not in any of its callers), did the object destruction in > unrealize to be symmetrical :) yes. I think it is a good pratice to unparent/free the object in the same file it was allocated. Thanks, C.
On Wed, Apr 12, 2017 at 01:45:07PM +0530, Bharata B Rao wrote: > Recent commits that re-organized ICPState object missed to destroy > the object when CPU is unrealized. Fix this so that CPU unplug > doesn't abort QEMU. > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> Applied to ppc-for-2.10, thanks. > --- > hw/ppc/spapr_cpu_core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > index 2e689b5..4389ef4 100644 > --- a/hw/ppc/spapr_cpu_core.c > +++ b/hw/ppc/spapr_cpu_core.c > @@ -127,6 +127,7 @@ static void spapr_cpu_core_unrealizefn(DeviceState *dev, Error **errp) > PowerPCCPU *cpu = POWERPC_CPU(cs); > > spapr_cpu_destroy(cpu); > + object_unparent(cpu->intc); > cpu_remove_sync(cs); > object_unparent(obj); > }
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index 2e689b5..4389ef4 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -127,6 +127,7 @@ static void spapr_cpu_core_unrealizefn(DeviceState *dev, Error **errp) PowerPCCPU *cpu = POWERPC_CPU(cs); spapr_cpu_destroy(cpu); + object_unparent(cpu->intc); cpu_remove_sync(cs); object_unparent(obj); }
Recent commits that re-organized ICPState object missed to destroy the object when CPU is unrealized. Fix this so that CPU unplug doesn't abort QEMU. Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> --- hw/ppc/spapr_cpu_core.c | 1 + 1 file changed, 1 insertion(+)