[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 03/13] migration: Use vmstate_register_any() for isa-ide
From: |
Juan Quintela |
Subject: |
Re: [PATCH v2 03/13] migration: Use vmstate_register_any() for isa-ide |
Date: |
Tue, 24 Oct 2023 12:55:47 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.3 (gnu/linux) |
Thomas Huth <thuth@redhat.com> wrote:
> 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.
Ide maintainer can do your change if he likes it. It is outside of my
understanding to accept it or not (and furthermore, it breaks migration
if you have only one device, with more than one it is already broken).
Later, Juan.
[PATCH v2 08/13] migration: vmstate_register() check that instance_id is valid, Juan Quintela, 2023/10/20
[PATCH v2 07/13] migration: Hack to maintain backwards compatibility for ppc, Juan Quintela, 2023/10/20
[PATCH v2 10/13] migration: Improve example and documentation of vmstate_register(), Juan Quintela, 2023/10/20
[PATCH v2 11/13] migration: Use vmstate_register_any() for audio, Juan Quintela, 2023/10/20
[PATCH v2 09/13] migration: Check in savevm_state_handler_insert for dups, Juan Quintela, 2023/10/20
[PATCH v2 12/13] migration: Use vmstate_register_any() for eeprom93xx, Juan Quintela, 2023/10/20