[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] spapr: Fix migration of PCI host bridges from q
From: |
Alexey Kardashevskiy |
Subject: |
Re: [Qemu-devel] [PATCH] spapr: Fix migration of PCI host bridges from qemu-2.7 |
Date: |
Wed, 9 Nov 2016 23:39:36 +1100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 |
On 09/11/16 23:19, David Gibson wrote:
> On Wed, Nov 09, 2016 at 04:14:25PM +1100, Alexey Kardashevskiy wrote:
>> On 09/11/16 14:45, 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.
>>>
>>> Although it would be technically possible to fix this in a way allowing
>>> backwards migration, that would leave an ugly legacy around indefinitely.
>>> This patch takes the simpler approach of bumping the migration version,
>>> dropping the unwise VMSTATE_EQUAL (and some equally unwise ones around it)
>>> and ignoring them on an incoming migration.
>>>
>>> Signed-off-by: David Gibson <address@hidden>
>>> ---
>>> hw/ppc/spapr_pci.c | 17 +++++++++++------
>>> 1 file changed, 11 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>>> index 7cde30e..7f1cc29 100644
>>> --- a/hw/ppc/spapr_pci.c
>>> +++ b/hw/ppc/spapr_pci.c
>>> @@ -1658,19 +1658,24 @@ static int spapr_pci_post_load(void *opaque, int
>>> version_id)
>>> return 0;
>>> }
>>>
>>> +static bool version_before_3(void *opaque, int version_id)
>>> +{
>>> + return version_id < 3;
>>> +}
>>> +
>>> static const VMStateDescription vmstate_spapr_pci = {
>>> .name = "spapr_pci",
>>> - .version_id = 2,
>>> + .version_id = 3,
>>> .minimum_version_id = 2,
>>> .pre_save = spapr_pci_pre_save,
>>> .post_load = spapr_pci_post_load,
>>> .fields = (VMStateField[]) {
>>> VMSTATE_UINT64_EQUAL(buid, sPAPRPHBState),
>>
>>
>> You could probably go one step further and get rid of @buid as well.
>
> I thought about it. buid at least is specified state that's
> vanishingly unlikely to change or disappear from the device. It also
> does serve to make sure that QOM instance matching - which is always a
> bit black magicy to me - is lining things up correctly.
afaict to fix matching properly, TYPE_SYS_BUS_DEVICE needs get_dev_path()
hook, just like spapr_vio_get_dev_name; otherwise SPAPR PHB in migration
always named as "spapr_pci" but yes, this would be much outside of the
scope of this patch :-/
>
>>
>> Nevertheless, this works,
>>
>> Reviewed-by: Alexey Kardashevskiy <address@hidden>
>>
>>
>>
>>
>>> - VMSTATE_UINT32_EQUAL(dma_liobn[0], sPAPRPHBState),
>>> - VMSTATE_UINT64_EQUAL(mem_win_addr, sPAPRPHBState),
>>> - VMSTATE_UINT64_EQUAL(mem_win_size, sPAPRPHBState),
>>> - VMSTATE_UINT64_EQUAL(io_win_addr, sPAPRPHBState),
>>> - VMSTATE_UINT64_EQUAL(io_win_size, sPAPRPHBState),
>>> + VMSTATE_UNUSED_TEST(version_before_3, sizeof(uint32_t) /*
>>> dma_liobn[0] */
>>> + + sizeof(uint64_t) /* mem_win_addr */
>>> + + sizeof(uint64_t) /* mem_win_size */
>>> + + sizeof(uint64_t) /* io_win_addr */
>>> + + sizeof(uint64_t) /* io_win_size */),
>>> VMSTATE_STRUCT_ARRAY(lsi_table, sPAPRPHBState, PCI_NUM_PINS, 0,
>>> vmstate_spapr_pci_lsi, struct spapr_pci_lsi),
>>> VMSTATE_INT32(msi_devs_num, sPAPRPHBState),
>>>
>>
>>
>
--
Alexey
signature.asc
Description: OpenPGP digital signature