Re: [Qemu-devel] [PULL 21/38] spapr: Add forgotten capability to migration stream

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

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

On Wed, May 22, 2019 at 09:58:29AM +0200, Greg Kurz wrote:
> On Wed, 22 May 2019 14:45:43 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > spapr machine capabilities are supposed to be sent in the migration stream
> > so that we can sanity check the source and destination have compatible
> > configuration.  Unfortunately, when we added the hpt-max-page-size
> > capability, we forgot to add it to the migration state.  This means that we
> > can generate spurious warnings when both ends are configured for large
> > pages, or potentially fail to warn if the source is configured for huge
> > pages, but the destination is not.
> > 
> > Fixes: 2309832afda "spapr: Maximum (HPT) pagesize property"
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > Reviewed-by: Cédric Le Goater <clg@kaod.org>
> > ---
> 
> Huh... we discussed that it was breaking backward migration:
> 
> https://lists.gnu.org/archive/html/qemu-ppc/2019-05/msg00330.html
> 
> So I'm a bit surprised to see this in the PR... is it intentional ?

Sod, no, I forgot to remove it from my tree.

Having been through the test cycle, I'd prefer not to hold up the PR
for this - as long as we fix it before the release we should be ok.

> 
> >  hw/ppc/spapr.c         | 1 +
> >  hw/ppc/spapr_caps.c    | 1 +
> >  include/hw/ppc/spapr.h | 1 +
> >  3 files changed, 3 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 8580a8dc67..bcae30ad26 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2125,6 +2125,7 @@ static const VMStateDescription vmstate_spapr = {
> >          &vmstate_spapr_cap_cfpc,
> >          &vmstate_spapr_cap_sbbc,
> >          &vmstate_spapr_cap_ibs,
> > +        &vmstate_spapr_cap_hpt_maxpagesize,
> >          &vmstate_spapr_irq_map,
> >          &vmstate_spapr_cap_nested_kvm_hv,
> >          &vmstate_spapr_dtb,
> > diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> > index 9b1c10baa6..658eb15a14 100644
> > --- a/hw/ppc/spapr_caps.c
> > +++ b/hw/ppc/spapr_caps.c
> > @@ -703,6 +703,7 @@ SPAPR_CAP_MIG_STATE(dfp, SPAPR_CAP_DFP);
> >  SPAPR_CAP_MIG_STATE(cfpc, SPAPR_CAP_CFPC);
> >  SPAPR_CAP_MIG_STATE(sbbc, SPAPR_CAP_SBBC);
> >  SPAPR_CAP_MIG_STATE(ibs, SPAPR_CAP_IBS);
> > +SPAPR_CAP_MIG_STATE(hpt_maxpagesize, SPAPR_CAP_HPT_MAXPAGESIZE);
> >  SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV);
> >  SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER);
> >  SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST);
> > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > index 7e32f309c2..9fc91c8f5e 100644
> > --- a/include/hw/ppc/spapr.h
> > +++ b/include/hw/ppc/spapr.h
> > @@ -849,6 +849,7 @@ extern const VMStateDescription vmstate_spapr_cap_dfp;
> >  extern const VMStateDescription vmstate_spapr_cap_cfpc;
> >  extern const VMStateDescription vmstate_spapr_cap_sbbc;
> >  extern const VMStateDescription vmstate_spapr_cap_ibs;
> > +extern const VMStateDescription vmstate_spapr_cap_hpt_maxpagesize;
> >  extern const VMStateDescription vmstate_spapr_cap_nested_kvm_hv;
> >  extern const VMStateDescription vmstate_spapr_cap_large_decr;
> >  extern const VMStateDescription vmstate_spapr_cap_ccf_assist;
> 

-- 
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: 833 bytes --]