[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3] ide: Fix memory leak in ide_register_restart
From: |
Ashijeet Acharya |
Subject: |
Re: [Qemu-devel] [PATCH v3] ide: Fix memory leak in ide_register_restart_cb() |
Date: |
Wed, 28 Sep 2016 10:45:57 +0530 |
On Wed, Sep 28, 2016 at 3:58 AM, John Snow <address@hidden> wrote:
>
>
> On 09/27/2016 12:53 PM, Ashijeet Acharya wrote:
>>
>> Fix a memory leak in ide_register_restart_cb() in hw/ide/core.c and add
>> idebus_unrealize() in hw/ide/qdev.c to have calls to
>> qemu_del_vm_change_state_handler() to deal with the dangling change
>> state handler during hot-unplugging ide devices which might lead to a
>> crash.
>>
>
> In the future, please rebase your patches on top of the current git master
> when you resend.
Yeah, sorry about that.
>
>
>> Signed-off-by: Ashijeet Acharya <address@hidden>
>> ---
>> Changes in v3:
>> -Use smaller conditional (bus->vmstate) only
>> ---
>> hw/ide/core.c | 2 +-
>> hw/ide/qdev.c | 11 +++++++++++
>> include/hw/ide/internal.h | 1 +
>> 3 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>> index 45b6df1..eecbb47 100644
>> --- a/hw/ide/core.c
>> +++ b/hw/ide/core.c
>> @@ -2582,7 +2582,7 @@ static void ide_restart_cb(void *opaque, int
>> running, RunState state)
>> void ide_register_restart_cb(IDEBus *bus)
>> {
>> if (bus->dma->ops->restart_dma) {
>> - qemu_add_vm_change_state_handler(ide_restart_cb, bus);
>> + bus->vmstate = qemu_add_vm_change_state_handler(ide_restart_cb,
>> bus);
>> }
>> }
>>
>> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
>> index 2eb055a..7e186bd 100644
>> --- a/hw/ide/qdev.c
>> +++ b/hw/ide/qdev.c
>> @@ -31,6 +31,7 @@
>> /* --------------------------------- */
>>
>> static char *idebus_get_fw_dev_path(DeviceState *dev);
>> +static void idebus_unrealize(DeviceState *qdev, Error **errp);
>>
>> static Property ide_props[] = {
>> DEFINE_PROP_UINT32("unit", IDEDevice, unit, -1),
>> @@ -44,6 +45,15 @@ static void ide_bus_class_init(ObjectClass *klass, void
>> *data)
>> k->get_fw_dev_path = idebus_get_fw_dev_path;
>> }
>>
>> +static void idebus_unrealize(DeviceState *qdev, Error **errp)
>> +{
>> + IDEBus *bus = DO_UPCAST(IDEBus, qbus, qdev->parent_bus);
>> +
>> + if (bus->vmstate) {
>> + qemu_del_vm_change_state_handler(bus->vmstate);
>
>
> Also in the future, keep an eye out for whitespace changes, too.
>
>> + }
>> +}
>> +
>> static const TypeInfo ide_bus_info = {
>> .name = TYPE_IDE_BUS,
>> .parent = TYPE_BUS,
>> @@ -355,6 +365,7 @@ static void ide_device_class_init(ObjectClass *klass,
>> void *data)
>> k->init = ide_qdev_init;
>> set_bit(DEVICE_CATEGORY_STORAGE, k->categories);
>> k->bus_type = TYPE_IDE_BUS;
>> + k->unrealize = idebus_unrealize;
>> k->props = ide_props;
>> }
>>
>> diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
>> index 7824bc3..2103261 100644
>> --- a/include/hw/ide/internal.h
>> +++ b/include/hw/ide/internal.h
>> @@ -480,6 +480,7 @@ struct IDEBus {
>> uint8_t retry_unit;
>> int64_t retry_sector_num;
>> uint32_t retry_nsector;
>> + VMChangeStateEntry *vmstate;
>> };
>>
>> #define TYPE_IDE_DEVICE "ide-device"
>>
>
> I made the minor rebase and whitespace edit in my tree, and with that:
>
> Reviewed-by: John Snow <address@hidden>
>
> Thanks, applied to my IDE tree:
Great! Thanks a lot.
Ashijeet
>
> https://github.com/jnsnow/qemu/commits/ide
> https://github.com/jnsnow/qemu.git
>
> --js