Re: [Qemu-devel] [PATCH qemu v14 08/18] spapr_iommu: Migrate full state

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2471 bytes --]

On Mon, Mar 21, 2016 at 06:46:56PM +1100, Alexey Kardashevskiy wrote:
> This adds @bus_offset, @page_shift, @enabled members to migration stream.
> These cannot change without dynamic DMA windows so no change in
> behavior is expected.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

I think you should combine this patch with the previous one.  They're
both simple, and the functions in the previous one check
tcet->enabled, which doesn't make a lot of sense if you're not
migrating that value.

The version bump here looks correct, but it will break migration of
(for example) a pseries-2.5 VM running under qemu-2.7 back into
qemu-2.5.  That sort of backwards migration isn't considered
essential, but it is nice to have (and it's something RH cares about
downstream).

So, if possible it would be preferable to do the migration in a
backwards compatible way.  The standard trick for that seems to be to
add an optional section with the extra info, and make the "needed"
function return true iff the parameters differ from the defaults.

> ---
> Changes:
> v14:
> * new to the series
> ---
>  hw/ppc/spapr_iommu.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> index 549cd94..5ea5948 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -180,7 +180,7 @@ static int spapr_tce_table_post_load(void *opaque, int version_id)
>  
>  static const VMStateDescription vmstate_spapr_tce_table = {
>      .name = "spapr_iommu",
> -    .version_id = 2,
> +    .version_id = 3,
>      .minimum_version_id = 2,
>      .pre_save = spapr_tce_table_pre_save,
>      .post_load = spapr_tce_table_post_load,
> @@ -189,6 +189,9 @@ static const VMStateDescription vmstate_spapr_tce_table = {
>          VMSTATE_UINT32_EQUAL(liobn, sPAPRTCETable),
>  
>          /* IOMMU state */
> +        VMSTATE_BOOL_V(enabled, sPAPRTCETable, 3),
> +        VMSTATE_UINT64_V(bus_offset, sPAPRTCETable, 3),
> +        VMSTATE_UINT32_V(page_shift, sPAPRTCETable, 3),
>          VMSTATE_UINT32(mig_nb_table, sPAPRTCETable),
>          VMSTATE_BOOL(bypass, sPAPRTCETable),
>          VMSTATE_VARRAY_UINT32_ALLOC(mig_table, sPAPRTCETable, nb_table, 0,

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]