1. Patchew
  2. QEMU
  3. [PATCH v2 00/13] migration: Check for duplicates on vmstate_register()
  4. View patch

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

Juan Quintela posted 13 patches 2 years, 2 months ago
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Stefan Berger <stefanb@linux.vnet.ibm.com>, Juan Quintela <quintela@redhat.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Leonardo Bras <leobras@redhat.com>, Corey Minyard <cminyard@mvista.com>, John Snow <jsnow@redhat.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Peter Maydell <peter.maydell@linaro.org>, Nicholas Piggin <npiggin@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, "Cédric Le Goater" <clg@kaod.org>, David Gibson <david@gibson.dropbear.id.au>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Stefan Weil <sw@weilnetz.de>, Jason Wang <jasowang@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Richard Henderson <richard.henderson@linaro.org>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Thomas Huth <thuth@redhat.com>, Samuel Thibault <samuel.thibault@ens-lyon.org>
There is a newer version of this series
[PATCH v2 03/13] migration: Use vmstate_register_any() for isa-ide
Posted by Juan Quintela 2 years, 2 months ago 
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);
 }
 
-- 
2.41.0
Re: [PATCH v2 03/13] migration: Use vmstate_register_any() for isa-ide
Posted by Thomas Huth 2 years, 2 months ago 
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
Re: [PATCH v2 03/13] migration: Use vmstate_register_any() for isa-ide
Posted by Juan Quintela 2 years, 2 months ago 
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.

The whole point of this series is to be able to test that there are no
duplicates.  Duplicates are one error when we do real migration.  How we
reach the goal of no duplicates doesn't matter to me.

Later, Juan.
Re: [PATCH v2 03/13] migration: Use vmstate_register_any() for isa-ide
Posted by Thomas Huth 2 years, 2 months ago 
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
Re: [PATCH v2 03/13] migration: Use vmstate_register_any() for isa-ide
Posted by Juan Quintela 2 years, 2 months ago 
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.

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.