qemu-devel
[Top][All Lists]
Advanced

[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 16:14:25 +1100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0

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.

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]