* [Qemu-devel] [PATCH v2] ide: Fix memory leak in ide_register_restart_cb()
@ 2016-09-21 17:53 Ashijeet Acharya
2016-09-21 19:38 ` John Snow
0 siblings, 1 reply; 5+ messages in thread
From: Ashijeet Acharya @ 2016-09-21 17:53 UTC (permalink / raw)
To: jsnow; +Cc: qemu-devel, Ashijeet Acharya
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.
Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
---
Changes in v2:
-v1 was corrupted at line 64
-Move idebus_unrealize() below ide_bus_class_init()
---
hw/ide/core.c | 2 +-
hw/ide/qdev.c | 13 +++++++++++++
include/hw/ide/internal.h | 1 +
3 files changed, 15 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..c94f9f8 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,17 @@ 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->dma->ops->restart_dma) {
+ if (bus->vmstate) {
+ qemu_del_vm_change_state_handler(bus->vmstate);
+ }
+ }
+}
+
static const TypeInfo ide_bus_info = {
.name = TYPE_IDE_BUS,
.parent = TYPE_BUS,
@@ -355,6 +367,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"
--
2.6.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2] ide: Fix memory leak in ide_register_restart_cb()
2016-09-21 17:53 [Qemu-devel] [PATCH v2] ide: Fix memory leak in ide_register_restart_cb() Ashijeet Acharya
@ 2016-09-21 19:38 ` John Snow
2016-09-22 5:47 ` Ashijeet Acharya
0 siblings, 1 reply; 5+ messages in thread
From: John Snow @ 2016-09-21 19:38 UTC (permalink / raw)
To: Ashijeet Acharya; +Cc: qemu-devel, Paolo Bonzini
On 09/21/2016 01: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.
>
Minor rebase issue, but it's trivially resolved.
> Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
> ---
> Changes in v2:
> -v1 was corrupted at line 64
> -Move idebus_unrealize() below ide_bus_class_init()
> ---
> hw/ide/core.c | 2 +-
> hw/ide/qdev.c | 13 +++++++++++++
> include/hw/ide/internal.h | 1 +
> 3 files changed, 15 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..c94f9f8 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,17 @@ 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->dma->ops->restart_dma) {
> + if (bus->vmstate) {
> + qemu_del_vm_change_state_handler(bus->vmstate);
> + }
> + }
> +}
> +
Naive question: I saw Paolo say that this should be conditional on
bus->dma->ops->restart_dma -- Why can't we just say if (bus->vmstate) ?
I see that we only allocate the change state handler when restart_dma is
present, but this makes this portion of the code look funny when if
(bus->vmstate) would be just as simple, no?
(Unless we can't rely on its NULL initialization or some such.)
> static const TypeInfo ide_bus_info = {
> .name = TYPE_IDE_BUS,
> .parent = TYPE_BUS,
> @@ -355,6 +367,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;
Hmm, I was going to complain and say that 'vmstate' is a generic name
for this field, but it's by far the most common name for this task. I cede!
> };
>
> #define TYPE_IDE_DEVICE "ide-device"
>
Seems good otherwise, thank you!
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2] ide: Fix memory leak in ide_register_restart_cb()
2016-09-21 19:38 ` John Snow
@ 2016-09-22 5:47 ` Ashijeet Acharya
2016-09-27 16:38 ` John Snow
0 siblings, 1 reply; 5+ messages in thread
From: Ashijeet Acharya @ 2016-09-22 5:47 UTC (permalink / raw)
To: John Snow; +Cc: QEMU Developers, Paolo Bonzini
On Thu, Sep 22, 2016 at 1:08 AM, John Snow <jsnow@redhat.com> wrote:
>
>
> On 09/21/2016 01: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.
>>
>
> Minor rebase issue, but it's trivially resolved.
>
>
>> Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
>> ---
>> Changes in v2:
>> -v1 was corrupted at line 64
>> -Move idebus_unrealize() below ide_bus_class_init()
>> ---
>> hw/ide/core.c | 2 +-
>> hw/ide/qdev.c | 13 +++++++++++++
>> include/hw/ide/internal.h | 1 +
>> 3 files changed, 15 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..c94f9f8 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,17 @@ 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->dma->ops->restart_dma) {
>> + if (bus->vmstate) {
>> + qemu_del_vm_change_state_handler(bus->vmstate);
>> + }
>> + }
>> +}
>> +
>
>
> Naive question: I saw Paolo say that this should be conditional on
> bus->dma->ops->restart_dma -- Why can't we just say if (bus->vmstate) ?
>
> I see that we only allocate the change state handler when restart_dma is
> present, but this makes this portion of the code look funny when if
> (bus->vmstate) would be just as simple, no?
>
I had a similar thought, because other pieces of code also do it in
"if (bus->vmstate)" manner but maybe I was missing on something
important and ended up coding how Paolo instructed me.
> (Unless we can't rely on its NULL initialization or some such.)
Maybe, but shouldn't the same logic apply elsewhere then?
>
>> static const TypeInfo ide_bus_info = {
>> .name = TYPE_IDE_BUS,
>> .parent = TYPE_BUS,
>> @@ -355,6 +367,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;
>
>
> Hmm, I was going to complain and say that 'vmstate' is a generic name for
> this field, but it's by far the most common name for this task. I cede!
>
>> };
>>
>> #define TYPE_IDE_DEVICE "ide-device"
>>
>
> Seems good otherwise, thank you!
Thanks!
Ashijeet
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2] ide: Fix memory leak in ide_register_restart_cb()
2016-09-22 5:47 ` Ashijeet Acharya
@ 2016-09-27 16:38 ` John Snow
2016-09-27 16:44 ` Ashijeet Acharya
0 siblings, 1 reply; 5+ messages in thread
From: John Snow @ 2016-09-27 16:38 UTC (permalink / raw)
To: Ashijeet Acharya, Paolo Bonzini; +Cc: QEMU Developers
On 09/22/2016 01:47 AM, Ashijeet Acharya wrote:
> On Thu, Sep 22, 2016 at 1:08 AM, John Snow <jsnow@redhat.com> wrote:
>>
>>
>> On 09/21/2016 01: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.
>>>
>>
>> Minor rebase issue, but it's trivially resolved.
>>
>>
>>> Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
>>> ---
>>> Changes in v2:
>>> -v1 was corrupted at line 64
>>> -Move idebus_unrealize() below ide_bus_class_init()
>>> ---
>>> hw/ide/core.c | 2 +-
>>> hw/ide/qdev.c | 13 +++++++++++++
>>> include/hw/ide/internal.h | 1 +
>>> 3 files changed, 15 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..c94f9f8 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,17 @@ 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->dma->ops->restart_dma) {
>>> + if (bus->vmstate) {
>>> + qemu_del_vm_change_state_handler(bus->vmstate);
>>> + }
>>> + }
>>> +}
>>> +
>>
>>
>> Naive question: I saw Paolo say that this should be conditional on
>> bus->dma->ops->restart_dma -- Why can't we just say if (bus->vmstate) ?
>>
>> I see that we only allocate the change state handler when restart_dma is
>> present, but this makes this portion of the code look funny when if
>> (bus->vmstate) would be just as simple, no?
>>
>
> I had a similar thought, because other pieces of code also do it in
> "if (bus->vmstate)" manner but maybe I was missing on something
> important and ended up coding how Paolo instructed me.
>
I think we can use the smaller conditional -- less prone to error that
way in case we change the conditional on how it was constructed. Let's
do that and I'll merge this for you finally.
>> (Unless we can't rely on its NULL initialization or some such.)
>
> Maybe, but shouldn't the same logic apply elsewhere then?
>
>>
>>> static const TypeInfo ide_bus_info = {
>>> .name = TYPE_IDE_BUS,
>>> .parent = TYPE_BUS,
>>> @@ -355,6 +367,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;
>>
>>
>> Hmm, I was going to complain and say that 'vmstate' is a generic name for
>> this field, but it's by far the most common name for this task. I cede!
>>
>>> };
>>>
>>> #define TYPE_IDE_DEVICE "ide-device"
>>>
>>
>> Seems good otherwise, thank you!
> Thanks!
>
> Ashijeet
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2] ide: Fix memory leak in ide_register_restart_cb()
2016-09-27 16:38 ` John Snow
@ 2016-09-27 16:44 ` Ashijeet Acharya
0 siblings, 0 replies; 5+ messages in thread
From: Ashijeet Acharya @ 2016-09-27 16:44 UTC (permalink / raw)
To: John Snow; +Cc: Paolo Bonzini, QEMU Developers
On Tue, Sep 27, 2016 at 10:08 PM, John Snow <jsnow@redhat.com> wrote:
>
>
> On 09/22/2016 01:47 AM, Ashijeet Acharya wrote:
>>
>> On Thu, Sep 22, 2016 at 1:08 AM, John Snow <jsnow@redhat.com> wrote:
>>>
>>>
>>>
>>> On 09/21/2016 01: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.
>>>>
>>>
>>> Minor rebase issue, but it's trivially resolved.
>>>
>>>
>>>> Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
>>>> ---
>>>> Changes in v2:
>>>> -v1 was corrupted at line 64
>>>> -Move idebus_unrealize() below ide_bus_class_init()
>>>> ---
>>>> hw/ide/core.c | 2 +-
>>>> hw/ide/qdev.c | 13 +++++++++++++
>>>> include/hw/ide/internal.h | 1 +
>>>> 3 files changed, 15 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..c94f9f8 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,17 @@ 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->dma->ops->restart_dma) {
>>>> + if (bus->vmstate) {
>>>> + qemu_del_vm_change_state_handler(bus->vmstate);
>>>> + }
>>>> + }
>>>> +}
>>>> +
>>>
>>>
>>>
>>> Naive question: I saw Paolo say that this should be conditional on
>>> bus->dma->ops->restart_dma -- Why can't we just say if (bus->vmstate) ?
>>>
>>> I see that we only allocate the change state handler when restart_dma is
>>> present, but this makes this portion of the code look funny when if
>>> (bus->vmstate) would be just as simple, no?
>>>
>>
>> I had a similar thought, because other pieces of code also do it in
>> "if (bus->vmstate)" manner but maybe I was missing on something
>> important and ended up coding how Paolo instructed me.
>>
>
> I think we can use the smaller conditional -- less prone to error that way
> in case we change the conditional on how it was constructed. Let's do that
> and I'll merge this for you finally.
Sure, I will send v3 ASAP.
Ashijeet
>
>
>>> (Unless we can't rely on its NULL initialization or some such.)
>>
>>
>> Maybe, but shouldn't the same logic apply elsewhere then?
>>
>>>
>>>> static const TypeInfo ide_bus_info = {
>>>> .name = TYPE_IDE_BUS,
>>>> .parent = TYPE_BUS,
>>>> @@ -355,6 +367,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;
>>>
>>>
>>>
>>> Hmm, I was going to complain and say that 'vmstate' is a generic name for
>>> this field, but it's by far the most common name for this task. I cede!
>>>
>>>> };
>>>>
>>>> #define TYPE_IDE_DEVICE "ide-device"
>>>>
>>>
>>> Seems good otherwise, thank you!
>>
>> Thanks!
>>
>> Ashijeet
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-09-27 16:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-21 17:53 [Qemu-devel] [PATCH v2] ide: Fix memory leak in ide_register_restart_cb() Ashijeet Acharya
2016-09-21 19:38 ` John Snow
2016-09-22 5:47 ` Ashijeet Acharya
2016-09-27 16:38 ` John Snow
2016-09-27 16:44 ` Ashijeet Acharya
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).