Re: [PATCH v2 03/13] migration: Use vmstate_register_any() for isa-ide

[Thomas Huth <thuth@redhat.com>]

On 20/10/2023 21.42, Juan Quintela wrote:
> Thomas Huth <thuth@redhat.com> wrote:
>> On 20/10/2023 11.07, Juan Quintela wrote:
>>> Otherwise qom-test fails.
>>> ok 4 /i386/qom/x-remote
>>> qemu-system-i386: savevm_state_handler_insert: Detected duplicate SaveStateEntry: id=isa-ide, instance_id=0x0
>>> Broken pipe
>>> ../../../../../mnt/code/qemu/full/tests/qtest/libqtest.c:195: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0)
>>> Aborted (core dumped)
>>> $
>>> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>>> ---
>>>    hw/ide/isa.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>> diff --git a/hw/ide/isa.c b/hw/ide/isa.c
>>> index 95053e026f..ea60c08116 100644
>>> --- a/hw/ide/isa.c
>>> +++ b/hw/ide/isa.c
>>> @@ -73,7 +73,7 @@ static void isa_ide_realizefn(DeviceState *dev, Error **errp)
>>>        ide_bus_init(&s->bus, sizeof(s->bus), dev, 0, 2);
>>>        ide_init_ioport(&s->bus, isadev, s->iobase, s->iobase2);
>>>        ide_bus_init_output_irq(&s->bus, isa_get_irq(isadev, s->irqnum));
>>> -    vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_isa, s);
>>> +    vmstate_register_any(VMSTATE_IF(dev), &vmstate_ide_isa, s);
>>>        ide_bus_register_restart_cb(&s->bus);
>>>    }
>>
>> Would it make sense to use another unique ID of the device instead? E.g.:
>>
>> diff a/hw/ide/isa.c b/hw/ide/isa.c
>> --- a/hw/ide/isa.c
>> +++ b/hw/ide/isa.c
>> @@ -73,7 +73,9 @@ static void isa_ide_realizefn(DeviceState *dev, Error **errp)
>>       ide_bus_init(&s->bus, sizeof(s->bus), dev, 0, 2);
>>       ide_init_ioport(&s->bus, isadev, s->iobase, s->iobase2);
>>       ide_bus_init_output_irq(&s->bus, isa_get_irq(isadev, s->irqnum));
>> -    vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_isa, s);
>> +    vmstate_register(VMSTATE_IF(dev),
>> +                     object_property_get_int(OBJECT(dev), "irq", &error_abort),
>> +                     &vmstate_ide_isa, s);
>>       ide_bus_register_restart_cb(&s->bus);
>>   }
>>     Thomas
> 
> Ide is not my part of expertise.
> But anything that is different for each instantance is going to be good
> for me.

It's not really my turf either ... ok, so unless the IDE maintainer speaks 
up, I think it's maybe best if you continue with your "_any" patch.

  Thomas