On Thu, Jun 01, 2017 at 02:54:48PM +1000, David Gibson wrote:
> On Wed, May 31, 2017 at 04:56:44PM +0530, Bharata B Rao wrote:
> > Add a "no HPT" encoding (using value -1) to the HTAB migration
> > stream (in the place of HPT size) when the guest doesn't allocate HPT.
> > This will help the target side to match target HPT with the source HPT
> > and thus enable successful migration.
> >
> > A few more fixes to enable TCG migration to work correctly are also
> > included in this commit:
> >
> > - HTAB savevm handlers have a few asserts on kvm_enabled() when
> > spapr->htab != 0. Convert these into conditional checks as it is now
> > possible to have no HTAB with TCG radix guests.
> > - htab_save_setup() asserts for kvm_enabled() when spapr->htab != 0.
> > Remove this as we can't assert this for TCG radix guests.
> >
> > Suggested-by: David Gibson <da...@gibson.dropbear.id.au>
> > [no HPT encoding suggestion]
> > Signed-off-by: Bharata B Rao <bhar...@linux.vnet.ibm.com>
>
> Looks basically ok, but there are still some details to address.
>
> > ---
> > hw/ppc/spapr.c | 31 +++++++++++++++++--------------
> > 1 file changed, 17 insertions(+), 14 deletions(-)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index ab3aab1..b589ed4 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1559,17 +1559,18 @@ static int htab_save_setup(QEMUFile *f, void
> > *opaque)
> > {
> > sPAPRMachineState *spapr = opaque;
> >
> > - /* "Iteration" header */
> > - qemu_put_be32(f, spapr->htab_shift);
> > + /* "Iteration" header: no-HPT or HPT size encoding */
> > + if (!spapr->htab_shift) {
> > + qemu_put_be32(f, -1);
>
> We're already using htab_shift == 0 to represent no HPT in the runtime
> structure; we might as well do the same on the wire. As a bonus it
> slightly simplifies the logic here.
Non-zero value of iteration header (which is htab_shift) results in
htab_load() at the target to reallocate HTAB.
zero value of iteration header is used by htab_save_iterate() and
htab_save_complete() to tell htab_load() not to freshly allocate HTAB
at the target.
Hence we can't use 0 value to mean no-HPT.
I have addressed the rest of the comments on asserts by ensuring that
those code paths are taken only when HPT is present. v5 has those
changes.
Regards,
Bharata.
