[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] ide: core: add cleanup function
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] ide: core: add cleanup function |
Date: |
Wed, 15 Feb 2017 14:53:52 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 |
On 02/15/2017 04:26 AM, Li Qiang wrote:
> Hello,
>
> 2017-02-15 7:30 GMT+08:00 John Snow <address@hidden
> <mailto:address@hidden>>:
>
>
>
> On 02/09/2017 08:22 PM, Li Qiang wrote:
> > Hello John,
> >
> > 2017-02-10 3:42 GMT+08:00 John Snow <address@hidden
> <mailto:address@hidden>
> > <mailto:address@hidden <mailto:address@hidden>>>:
> >
> >
> >
> > On 02/09/2017 02:04 AM, Li Qiang wrote:
> > > As the pci ahci can be hotplug and unplug, in the ahci unrealize
> > > function it should free all the resource once allocated in the
> > > realized function. This patch adds two cleanup function.
> > >
> > > Signed-off-by: Li Qiang <address@hidden
> <mailto:address@hidden> <mailto:address@hidden
> <mailto:address@hidden>>>
> > > ---
> > > hw/ide/core.c | 21 +++++++++++++++++++++
> > > include/hw/ide/internal.h | 2 ++
> > > 2 files changed, 23 insertions(+)
> > >
> > > diff --git a/hw/ide/core.c b/hw/ide/core.c
> > > index 43709e5..8fe5896 100644
> > > --- a/hw/ide/core.c
> > > +++ b/hw/ide/core.c
> > > @@ -2586,6 +2586,13 @@ void ide_register_restart_cb(IDEBus *bus)
> > > }
> > > }
> > >
> > > +void ide_unregister_restart_cb(IDEBus *bus)
> > > +{
> > > + if (bus->dma->ops->restart_dma) {
> > > + qemu_del_vm_change_state_handler(bus->vmstate);
> > > + }
> > > +}
> > > +
> >
> > Doesn't this conflict with qdev.c's idebus_unrealize call?
> >
> >
> > As far as I can see, No conflict. Let's get a confirmation.
> >
> > Hello Paolo,
> >
> > Does this patch have any conflict/affect with the qdev?
> >
> > Thanks.
> >
>
> They're both deleting the same VMstate handler, so as far as I can see
> this is a redundant call.
>
>
> IIUC, the idebus_unrealize in qdev.c is for qdev model.
> But the added is for qom model.
> For example, if you use 'device_add ahci,id=ahci'/'device_del ahci' in
> the qmp.
>
> The qemu will call 'pci_ich9_ahci_realize'/'pci_ich9_uninit'.
>
>
>
I'm sorry, I still don't understand. Do you have some reproducer or case
where I can verify that this leaks?
It doesn't look as if you can hot-add or hot-remove an AHCI device right
now anyway, have you tested this?
Further, if the two calls AREN'T in conflict, I'd rather find some
cleanup mechanism that handles all the unrealize/uninit cases together
instead of having separate cleanup pathways.
--js
Re: [Qemu-devel] [PATCH 1/2] ide: core: add cleanup function, John Snow, 2017/02/28
[Qemu-devel] [PATCH 2/2] ide: ahci: call cleanup function in ahci unit, Li Qiang, 2017/02/09