On Thu, Aug 11, 2016 at 1:50 PM, Ashijeet Acharya <ashijeetacha...@gmail.com> wrote: > On Wed, Aug 10, 2016 at 11:12 AM, Ashijeet Acharya > <ashijeetacha...@gmail.com> wrote: >> On Tue, Aug 9, 2016 at 11:48 PM, John Snow <js...@redhat.com> 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 >>>> <ashijeetacha...@gmail.com> 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. >>>
I think we should do s->vmstate = qemu_add_vm_change_state_handler(ide_restart_cb, bus); instead of qemu_add_vm_change_state_handler(ide_restart_cb, bus); in ide_register_restart_cb() in hw/ide/core.c to store the returned pointer to memory to avoid a possible memory leak I guess and introduce a VMChangeStateEntry field in struct AHCIState to handle this. Same can then further be used with qemu_del_vm_change_state_handler() in ahci_unrealize() to free things up. Ashijeet >>> >>>>> Please tell me if I am on the right path and correct me if I am wrong. >>>>> >>>>> Thanks >>>>> Ashijeet >>> >>> >>> -- >>> —js
