Re: [Qemu-devel] Dangling change state handler while hot unplugging ahci adapter

[Ashijeet Acharya]

On Wed, Aug 10, 2016 at 11:12 AM, Ashijeet Acharya
<address@hidden> wrote:
> On Tue, Aug 9, 2016 at 11:48 PM, John Snow <address@hidden> wrote:
>>
>>
>> On 08/09/2016 01:16 PM, Ashijeet Acharya wrote:
>>>
>>> Hi again,
>>> I am still waiting for some guidance...Can I please get some help with
>>> this?
>>>
>>> Also.. I tried hotplugging an AHCI adapter but got the following error:
>>> Bus 'ahci.0' does not support hotplugging
>>>
>>> Steps I followed:
>>>
>>> 1. launch vm with ahci enabled
>>> 2. (qemu) drive_add 0
>>> file=test.qcow2,cache=none,if=none,id=disk2,format=qcow2
>>> OK
>>> 3. (qemu) device_add ide-hd,bus=ahci.0,id=ahci-disk,drive=disk2
>>> Bus 'ahci.0' does not support hotplugging
>>>
>>> What am I doing wrong?
>>>
>>> Thanks
>>> Ashijeet
>>>
>>
>> I think you are confusing hotplugging the AHCI adapter with hotplugging an
>> IDE drive into an AHCI adapter.
>>
>> IDE/ATA/PATA (A rose by any other name...) drives do not support hotplugging
>> (hence "Bus 'ahci.0' does not support hotplugging").
>>
>> SATA drives do in theory, but it hasn't been implemented yet.
>>
>> That's probably a little more involved than a Bite Sized Task, but it's
>> something that I'd be willing to review if you embarked upon such a task.

Sure... I can work upon that after I complete this bitesized task and
get more acquainted with this part of the code.

Ashijeet Acharya
>>
>>> On Sat, Aug 6, 2016 at 11:21 PM, Ashijeet Acharya
>>> <address@hidden> wrote:
>>>>
>>>> Hi,
>>>>
>>>> I was working on a patch regarding a device lifecycle bitesize task
>>>> and I wanted to clear my queries about what the task is exactly.
>>>>
>>>> Do I need to create a new function ahci_unrealize() in the
>>>> hw/ide/ahci.c file which calls for qemu_del_vm_change_state_handler()
>>>> to free the handler at the time of ahci hot unplug.
>>>>
>>
>> Sounds about right. grep around for other instances of
>> qemu_del_vm_change_state_handler and try to code by example.
>
> Alright..thanks!
>
> I found out that ahci_realize() calls for ide_register_restart_cb()
> which has qemu_add_vm_change_state_handler() but the returned value
> from qemu_add_vm_change_state_handler() does not get assigned to any
> VMChangeStateEntry pointer. This should lead (I guess..) to a memory
> leak! Maybe introducing a VMChangeStateEntry field in struct AHCIState
> help and can be used here. This later can also be passed to
> qemu_del_vm_change_state_handler() to free things up.
>>
>>
>>>> Please tell me if I am on the right path and correct me if I am wrong.
>>>>
>>>> Thanks
>>>> Ashijeet
>>
>>
>> --
>> —js