Re: [PATCH 3/3] isa/piix4: Resolve global variables

[BB <shentey@gmail.com>]

Am 30. Januar 2022 23:53:42 MEZ schrieb "Philippe Mathieu-Daudé" <f4bug@amsat.org>:
>On 14/1/22 14:36, Peter Maydell wrote:
>> On Wed, 12 Jan 2022 at 22:02, Bernhard Beschow <shentey@gmail.com> wrote:
>>>
>>> Now that piix4_set_irq's opaque parameter references own PIIX4State,
>>> piix4_dev becomes redundant and pci_irq_levels can be moved into PIIX4State.
>>>
>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>> ---
>>>   hw/isa/piix4.c                | 22 +++++++++-------------
>>>   include/hw/southbridge/piix.h |  2 --
>>>   2 files changed, 9 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
>>> index a31e9714cf..964e09cf7f 100644
>>> --- a/hw/isa/piix4.c
>>> +++ b/hw/isa/piix4.c
>>> @@ -39,14 +39,14 @@
>>>   #include "sysemu/runstate.h"
>>>   #include "qom/object.h"
>>>
>>> -PCIDevice *piix4_dev;
>>> -
>>>   struct PIIX4State {
>>>       PCIDevice dev;
>>>       qemu_irq cpu_intr;
>>>       qemu_irq *isa;
>>>       qemu_irq i8259[ISA_NUM_IRQS];
>>>
>>> +    int pci_irq_levels[PIIX_NUM_PIRQS];
>>> +
>> 
>> I wondered how we were migrating this state, and the answer
>> seems to be that we aren't (and weren't before, when it was
>> a global variable, so this is a pre-existing bug).
>
>Indeed the migrated VM starts with PCI IRQ levels zeroed.
>
>> Does the malta platform support migration save/load?
>
>Maybe a "best effort" support, but not versioned machines.
>
>> We should probably add this field to the vmstate struct
>> (which will be a migration compatibility break, which is OK
>> as the malta board isn't versioned.)
>
>Yeah good catch.
>
>Bernhard, do you mind adding it?

Sure, I'll give it a try. Shall I submit a v2 of this patch series then? If so, would it be ok to change the topic of the cover letter or would that be confusing?

Last but not least: How to treat the version_id and the version parameters of the new and existing fields?

Regards,

Bernhard.