Re: [Qemu-ppc] [PATCH for-2.8 0/3] spapr: fix breakage of memory unplug after migration

[David Gibson]

On Thu, Nov 17, 2016 at 07:40:24PM -0600, Michael Roth wrote:
> These patches are based on David's ppc-for-2.8 tree, and are also
> available from:
> 
>   https://github.com/mdroth/qemu/commits/spapr-cas-migration
> 
> Currently, memory hotplugged to a pseries guest cannot be removed after
> the guest has been migrated. This is due to 2 issues:
> 
> 1) The coldplugged state of memory on the target side is one where the
>    corresponding DRC's allocation state is:
> 
>      allocation_state == unallocated,
>      awaiting_allocation == true,
> 
>    When the guest attempts to unplug memory on the target side, it first
>    checks that allocation_state == allocated. If we fix this, the guest
>    can successfully notify QEMU of completion on it's end, but then the
>    DRC code sees that awaiting_allocation == true, so it defers the
>    finalizing of the LMB and corresponding DIMM since it assumes that
>    the DIMM must have been previously allocated before it can be removed.
> 
>    To address this, we pull in patches 1-2 from Jian Jun's DRC migration
>    series:
> 
>      https://lists.gnu.org/archive/html/qemu-ppc/2016-10/msg00048.html
> 
>    with some minor changes relating to prior review comments, and
>    the addition of migrating the DRC's awaiting_allocation value, which
>    wasn't part of the original patch. This doesn't address the full scope
>    of the issues Jian Jun was looking at (involving synchronizing state
>    when migration occurs during fairly small race windows), just this
>    particular case, which is more user visible since the time window is
>    indefinite.
> 
> 2) The ability to unplug memory is gated on the QEMU side by a check as
>    to whether or not support for newer-style hotplug events was negotiated
>    via CAS during boot. The check is performed by checking the corresponding
>    entry in the sPAPROptionVector structure. However, since this value isn't
>    migrated currently, we are unable to unplug until after the guest reboots.
> 
>    We address that here by adding migration support for sPAPROptionVectors,
>    and including the CAS-negotiated vector as part of the migration stream
>    for any cases where we advertise newer-style hotplug event support to
>    the guest.
> 
> David,
> 
> These fixes ended up going out much later than planned. I'm not sure
> if you're planning another pull for 2.8 or not, and realize there are
> some patches here not specifically pseries-related so it's
> understandable if we opt to pursue these for 2.9/2.8.1 instead. But if
> possible I'm hoping to get these in so that the memory unplug
> support is fully functional for 2.8.

Yeah, I'm still expecting to push a few bugfixes in before 2.8.  So,
I've merged these patches into ppc-for-2.8 (fixing a couple of trivial
style nits along the way).  I have a couple of comments that I'll make
on the patches, but they're not important enough to stop these going
in ASAP.

Unfortunately, of course, this is not the only migration breakage we
have at the moment.  I'm presently wrestling with both breakage due to
changes in the insns_flags masks, and due to the reworking of the mmio
windows for the PHB.

> 
> 
>  hw/core/qdev.c              |  6 +++++-
>  hw/ppc/spapr.c              | 68 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/ppc/spapr_drc.c          | 70 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/ppc/spapr_ovec.c         | 12 ++++++++++++
>  hw/ppc/spapr_pci.c          | 22 ++++++++++++++++++++++
>  include/hw/ppc/spapr_drc.h  |  9 +++++++++
>  include/hw/ppc/spapr_ovec.h |  4 ++++
>  include/hw/qdev-core.h      |  9 +++++++++
>  migration/savevm.c          |  4 ++--
>  9 files changed, 201 insertions(+), 3 deletions(-)
> 

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

signature.asc
Description: PGP signature