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

[Michael S. Tsirkin]

On Thu, Feb 10, 2022 at 12:16:34AM +0100, BB wrote:
> 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?

It's ok to change the subject of the cover letter.

> Last but not least: How to treat the version_id and the version parameters of 
> the new and existing fields?
> 
> Regards,
> 
> Bernhard.