On Mon, Jul 17, 2017 at 09:54:45PM +0200, Laurent Vivier wrote:
> On 30/06/2017 12:46, David Gibson wrote:
> > From: Bharata B Rao <bharata@linux.vnet.ibm.com>
> >
> > 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.
> >
> > Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> > hw/ppc/spapr.c | 29 +++++++++++++++++++++++++----
> > 1 file changed, 25 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 52f4e72..f3e0b9b 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1560,13 +1560,19 @@ static int htab_save_setup(QEMUFile *f, void *opaque)
> > sPAPRMachineState *spapr = opaque;
> >
> > /* "Iteration" header */
> > - qemu_put_be32(f, spapr->htab_shift);
> > + if (!spapr->htab_shift) {
> > + qemu_put_be32(f, -1);
> > + } else {
> > + qemu_put_be32(f, spapr->htab_shift);
> > + }
> >
> > if (spapr->htab) {
> > spapr->htab_save_index = 0;
> > spapr->htab_first_pass = true;
> > } else {
> > - assert(kvm_enabled());
> > + if (spapr->htab_shift) {
> > + assert(kvm_enabled());
> > + }
> > }
> >
> >
> > @@ -1712,7 +1718,12 @@ static int htab_save_iterate(QEMUFile *f, void *opaque)
> > int rc = 0;
> >
> > /* Iteration header */
> > - qemu_put_be32(f, 0);
> > + if (!spapr->htab_shift) {
> > + qemu_put_be32(f, -1);
> > + return 0;
> > + } else {
> > + qemu_put_be32(f, 0);
> > + }
> >
> > if (!spapr->htab) {
> > assert(kvm_enabled());
> > @@ -1746,7 +1757,12 @@ static int htab_save_complete(QEMUFile *f, void *opaque)
> > int fd;
> >
> > /* Iteration header */
> > - qemu_put_be32(f, 0);
> > + if (!spapr->htab_shift) {
> > + qemu_put_be32(f, -1);
> > + return 0;
> > + } else {
> > + qemu_put_be32(f, 0);
> > + }
> >
> > if (!spapr->htab) {
> > int rc;
> > @@ -1790,6 +1806,11 @@ static int htab_load(QEMUFile *f, void *opaque, int version_id)
> >
> > section_hdr = qemu_get_be32(f);
> >
> > + if (section_hdr == -1) {
> > + spapr_free_hpt(spapr);
> > + return 0;
> > + }
> > +
> > if (section_hdr) {
> > Error *local_err = NULL;
> >
> >
>
> It seems there is a bug in this patch: when using from QEMU console the
> command "savevm", it never stops and the qcow2 image grows without limit.
>
> I think this is because htab_save_iterate() and htab_save_complete()
> should mark the end of the sequence (the empty one, -1) by returning 1
> and not 0 (see kvmppc_save_htab()).
Ah, yes, I think you're right. The end condition is one of the subtle
differences between the savevm and migration paths.
> This fixes the problem for me:
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 970093e..fa01511 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1827,7 +1827,7 @@ static int htab_save_iterate(QEMUFile *f, void
> *opaque)
> /* Iteration header */
> if (!spapr->htab_shift) {
> qemu_put_be32(f, -1);
> - return 0;
> + return 1;
> } else {
> qemu_put_be32(f, 0);
> }
> @@ -1866,7 +1866,7 @@ static int htab_save_complete(QEMUFile *f, void
> *opaque)
> /* Iteration header */
> if (!spapr->htab_shift) {
> qemu_put_be32(f, -1);
> - return 0;
> + return 1;
> } else {
> qemu_put_be32(f, 0);
> }
Can you polish this up and submit for merge, please?
--
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