Re: [Qemu-devel] [PATCH for-2.8 3/3] spapr: migration support for CAS-negotiated option vectors

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

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

On Fri, Nov 18, 2016 at 10:08:59AM -0600, Michael Roth wrote:
> Quoting Michael Roth (2016-11-17 19:40:27)
> > With the additional of the OV5_HP_EVT option vector, we now have
> > certain functionality (namely, memory unplug) that checks at run-time
> > for whether or not the guest negotiated the option via CAS. Because
> > we don't currently migrate these negotiated values, we are unable
> > to unplug memory from a guest after it's been migrated until after
> > the guest is rebooted and CAS-negotiation is repeated.
> > 
> > This patch fixes this by adding CAS-negotiated options to the
> > migration stream. We do this using a subsection, since the
> > negotiated value of OV5_HP_EVT is the only option currently needed
> > to maintain proper functionality for a running guest.
> > 
> > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > ---
> >  hw/ppc/spapr.c              | 68 +++++++++++++++++++++++++++++++++++++++++++++
> >  hw/ppc/spapr_ovec.c         | 12 ++++++++
> >  include/hw/ppc/spapr_ovec.h |  4 +++
> >  3 files changed, 84 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 0cbab24..9e08aed 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1267,6 +1267,70 @@ static bool version_before_3(void *opaque, int version_id)
> >      return version_id < 3;
> >  }
> > 
> > +static bool spapr_ov5_cas_needed(void *opaque)
> > +{
> > +    sPAPRMachineState *spapr = opaque;
> > +    sPAPROptionVector *ov5_mask = spapr_ovec_new();
> > +    sPAPROptionVector *ov5_legacy = spapr_ovec_new();
> > +    sPAPROptionVector *ov5_removed = spapr_ovec_new();
> > +    bool cas_needed;
> > +
> > +    /* Prior to the introduction of sPAPROptionVector, we had two option
> > +     * vectors we dealt with: OV5_FORM1_AFFINITY, and OV5_DRCONF_MEMORY.
> > +     * Both of these options encode machine topology into the device-tree
> > +     * in such a way that the now-booted OS should still be able to interact
> > +     * appropriately with QEMU regardless of what options were actually
> > +     * negotiatied on the source side.
> > +     *
> > +     * As such, we can avoid migrating the CAS-negotiated options if these
> > +     * are the only options available on the current machine/platform.
> > +     * Since these are the only options available for pseries-2.7 and
> > +     * earlier, this allows us to maintain old->new/new->old migration
> > +     * compatibility.
> > +     *
> > +     * For QEMU 2.8+, there are additional CAS-negotiatable options available
> > +     * via default pseries-2.8 machines and explicit command-line parameters.
> > +     * Some of these options, like OV5_HP_EVT, *do* require QEMU to be aware
> > +     * of the actual CAS-negotiated values to continue working properly. For
> > +     * example, availability of memory unplug depends on knowing whether
> > +     * OV5_HP_EVT was negotiated via CAS.
> > +     *
> > +     * Thus, for any cases where the set of available CAS-negotiatable
> > +     * options extends beyond OV5_FORM1_AFFINITY and OV5_DRCONF_MEMORY, we
> > +     * include the CAS-negotiated options in the migration stream.
> > +     */
> > +    spapr_ovec_set(ov5_mask, OV5_FORM1_AFFINITY);
> > +    spapr_ovec_set(ov5_mask, OV5_DRCONF_MEMORY);
> > +
> > +    /* spapr_ovec_diff returns true if bits were removed. we avoid using
> > +     * the mask itself since in the future it's possible "legacy" bits may be
> > +     * removed via machine options, which could generate a false positive
> > +     * that breaks migration.
> > +     */
> > +    spapr_ovec_intersect(ov5_legacy, spapr->ov5, ov5_mask);
> > +    cas_needed = spapr_ovec_diff(ov5_removed, spapr->ov5, ov5_legacy);
> > +
> > +    spapr_ovec_cleanup(ov5_mask);
> > +    spapr_ovec_cleanup(ov5_legacy);
> > +    spapr_ovec_cleanup(ov5_removed);
> > +
> > +    error_report("MIGRATION NEEDED: %d", cas_needed);
> 
> Argh, sorry, I just noticed this stray debug comment that slipped in.
> Would you prefer a v2, or just removing it in-tree?

I've fixed it in tree.  Thanks for pointing it out.

-- 
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 --]