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

[Michael Roth]

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 <address@hidden>
> ---
>  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?

> +
> +    return cas_needed;
> +}
> +
> +static const VMStateDescription vmstate_spapr_ov5_cas = {
> +    .name = "spapr_option_vector_ov5_cas",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = spapr_ov5_cas_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_STRUCT_POINTER_V(ov5_cas, sPAPRMachineState, 1,
> +                                 vmstate_spapr_ovec, sPAPROptionVector),
> +        VMSTATE_END_OF_LIST()
> +    },
> +};
> +
>  static const VMStateDescription vmstate_spapr = {
>      .name = "spapr",
>      .version_id = 3,
> @@ -1282,6 +1346,10 @@ static const VMStateDescription vmstate_spapr = {
>          VMSTATE_PPC_TIMEBASE_V(tb, sPAPRMachineState, 2),
>          VMSTATE_END_OF_LIST()
>      },
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_spapr_ov5_cas,
> +        NULL
> +    }
>  };
> 
>  static int htab_save_setup(QEMUFile *f, void *opaque)
> diff --git a/hw/ppc/spapr_ovec.c b/hw/ppc/spapr_ovec.c
> index c2a0d18..3eb1d59 100644
> --- a/hw/ppc/spapr_ovec.c
> +++ b/hw/ppc/spapr_ovec.c
> @@ -37,6 +37,17 @@
>   */
>  struct sPAPROptionVector {
>      unsigned long *bitmap;
> +    int32_t bitmap_size; /* only used for migration */
> +};
> +
> +const VMStateDescription vmstate_spapr_ovec = {
> +    .name = "spapr_option_vector",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_BITMAP(bitmap, sPAPROptionVector, 1, bitmap_size),
> +        VMSTATE_END_OF_LIST()
> +    }
>  };
> 
>  sPAPROptionVector *spapr_ovec_new(void)
> @@ -45,6 +56,7 @@ sPAPROptionVector *spapr_ovec_new(void)
> 
>      ov = g_new0(sPAPROptionVector, 1);
>      ov->bitmap = bitmap_new(OV_MAXBITS);
> +    ov->bitmap_size = OV_MAXBITS;
> 
>      return ov;
>  }
> diff --git a/include/hw/ppc/spapr_ovec.h b/include/hw/ppc/spapr_ovec.h
> index 6a06da3..355a344 100644
> --- a/include/hw/ppc/spapr_ovec.h
> +++ b/include/hw/ppc/spapr_ovec.h
> @@ -37,6 +37,7 @@
>  #define _SPAPR_OVEC_H
> 
>  #include "cpu.h"
> +#include "migration/vmstate.h"
> 
>  typedef struct sPAPROptionVector sPAPROptionVector;
> 
> @@ -64,4 +65,7 @@ sPAPROptionVector *spapr_ovec_parse_vector(target_ulong 
> table_addr, int vector);
>  int spapr_ovec_populate_dt(void *fdt, int fdt_offset,
>                             sPAPROptionVector *ov, const char *name);
> 
> +/* migration */
> +extern const VMStateDescription vmstate_spapr_ovec;
> +
>  #endif /* !defined (_SPAPR_OVEC_H) */
> -- 
> 1.9.1
> 
>