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

[Ashijeet Acharya]

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.
>
>> 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