On 23/11/16 11:17, David Gibson wrote:
> On Tue, Nov 22, 2016 at 07:17:27PM +1100, Alexey Kardashevskiy wrote:
>> On 21/11/16 16:31, David Gibson wrote:
>>> daa2369 "spapr_pci: Add a 64-bit MMIO window" subtly broke migration
>>> from qemu-2.7 to the current version. It split the device's MMIO
>>> window into two pieces for 32-bit and 64-bit MMIO.
>>>
>>> The patch included backwards compatibility code to convert the old
>>> property into the new format. However, the property value was also
>>> transferred in the migration stream and compared with a (probably
>>> unwise) VMSTATE_EQUAL. So, the "raw" value from 2.7 is compared to
>>> the new style converted value from (pre-)2.8 giving a mismatch and
>>> migration failure.
>>>
>>> Along with the actual field that caused the breakage, there are
>>> several other ill-advised VMSTATE_EQUAL()s. To fix forwards
>>> migration, we read the values in the stream into scratch variables and
>>> ignore them, instead of comparing for equality. To fix backwards
>>> migration, we populate those scratch variables in pre_save() with
>>> adjusted values to match the old behaviour.
>>>
>>> To permit the eventual possibility of removing this cruft from the
>>> stream, we only include these compatibility fields if a new
>>> 'pre-2.8-migration' property is set. We clear it on the pseries-2.8
>>> machine type, which obviously can't be migrated backwards, but set it
>>> on earlier machine type versions.
>>>
>>> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
>>> ---
>>> hw/ppc/spapr.c | 5 +++++
>>> hw/ppc/spapr_pci.c | 33 ++++++++++++++++++++++++++++-----
>>> include/hw/pci-host/spapr.h | 6 ++++++
>>> 3 files changed, 39 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index 775ad2e..c3269c7 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -2772,6 +2772,11 @@ DEFINE_SPAPR_MACHINE(2_8, "2.8", true);
>>> .driver = TYPE_POWERPC_CPU, \
>>> .property = "pre-2.8-migration", \
>>> .value = "on", \
>>> + }, \
>>> + { \
>>> + .driver = TYPE_SPAPR_PCI_HOST_BRIDGE, \
>>> + .property = "pre-2.8-migration", \
>>> + .value = "on", \
>>> },
>>>
>>> static void phb_placement_2_7(sPAPRMachineState *spapr, uint32_t index,
>>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>>> index e429c94..c62c1cb 100644
>>> --- a/hw/ppc/spapr_pci.c
>>> +++ b/hw/ppc/spapr_pci.c
>>> @@ -1590,6 +1590,8 @@ static Property spapr_phb_properties[] = {
>>> DEFINE_PROP_UINT64("pgsz", sPAPRPHBState, page_size_mask,
>>> (1ULL << 12) | (1ULL << 16)),
>>> DEFINE_PROP_UINT32("numa_node", sPAPRPHBState, numa_node, -1),
>>> + DEFINE_PROP_BOOL("pre-2.8-migration", sPAPRPHBState,
>>> + pre_2_8_migration, false),
>>> DEFINE_PROP_END_OF_LIST(),
>>> };
>>>
>>> @@ -1636,6 +1638,20 @@ static void spapr_pci_pre_save(void *opaque)
>>> sphb->msi_devs[i].key = *(uint32_t *) key;
>>> sphb->msi_devs[i].value = *(spapr_pci_msi *) value;
>>> }
>>> +
>>> + if (sphb->pre_2_8_migration) {
>>
>>
>> You check for pre_2_8_migration here but you do not in 3/5, what is the
>> difference?
>
> Uh.. where don't I in 3/5?Never mind, I did not sleep well and while switching between patches, I confused 1/5 (which does not have a check) with 3/5 (which does). My bad, sorry for the noise. I run some more tests today, just to make sure I slept well, all good :) -- Alexey
signature.asc
Description: OpenPGP digital signature
