[Qemu-devel] [PULL 04/60] Revert "spapr: support memory unplug for qtest"

David Gibson posted 60 patches 6 years, 11 months ago
Maintainers: "Marc-André Lureau" <marcandre.lureau@redhat.com>, Thomas Huth <thuth@redhat.com>, Alex Williamson <alex.williamson@redhat.com>, Jason Wang <jasowang@redhat.com>, Laurent Vivier <lvivier@redhat.com>, David Gibson <david@gibson.dropbear.id.au>, Fam Zheng <fam@euphon.net>, "Cédric Le Goater" <clg@kaod.org>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Paolo Bonzini <pbonzini@redhat.com>
[Qemu-devel] [PULL 04/60] Revert "spapr: support memory unplug for qtest"
Posted by David Gibson 6 years, 11 months ago
From: Greg Kurz <groug@kaod.org>

Commit b8165118f52c broke CPU hotplug tests for old machine types:

$ QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 ./tests/cpu-plug-test -m=slow
/ppc64/cpu-plug/pseries-3.1/device-add/2x3x1&maxcpus=6: OK
/ppc64/cpu-plug/pseries-2.12-sxxm/device-add/2x3x1&maxcpus=6: OK
/ppc64/cpu-plug/pseries-3.0/device-add/2x3x1&maxcpus=6: OK
/ppc64/cpu-plug/pseries-2.10/device-add/2x3x1&maxcpus=6: OK
/ppc64/cpu-plug/pseries-2.11/device-add/2x3x1&maxcpus=6: OK
/ppc64/cpu-plug/pseries-2.12/device-add/2x3x1&maxcpus=6: OK
/ppc64/cpu-plug/pseries-2.9/device-add/2x3x1&maxcpus=6: OK
/ppc64/cpu-plug/pseries-2.7/device-add/2x3x1&maxcpus=6: **
ERROR:/home/thuth/devel/qemu/hw/ppc/spapr_events.c:313:rtas_event_log_to_source: assertion failed: (source->enabled)
Broken pipe
/home/thuth/devel/qemu/tests/libqtest.c:143: kill_qemu() detected QEMU death from signal 6 (Aborted) (core dumped)
Aborted (core dumped)

The approach of faking the availability of OV5_HP_EVT causes the
code to assume the hotplug event source is enabled, which is wrong
for older machines.

This reverts commit b8165118f52ce5ee88565d3cec83d30374efdc96.

A subsequent patch will address the problem of CAS under qtest from
a different angle.

Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <155146875097.147873.1732264036668112686.stgit@bahia.lan>
Tested-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_ovec.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/hw/ppc/spapr_ovec.c b/hw/ppc/spapr_ovec.c
index 12510b236a..318bf33de4 100644
--- a/hw/ppc/spapr_ovec.c
+++ b/hw/ppc/spapr_ovec.c
@@ -16,7 +16,6 @@
 #include "qemu/bitmap.h"
 #include "exec/address-spaces.h"
 #include "qemu/error-report.h"
-#include "sysemu/qtest.h"
 #include "trace.h"
 #include <libfdt.h>
 
@@ -132,11 +131,6 @@ bool spapr_ovec_test(sPAPROptionVector *ov, long bitnr)
     g_assert(ov);
     g_assert(bitnr < OV_MAXBITS);
 
-    /* support memory unplug for qtest */
-    if (qtest_enabled() && bitnr == OV5_HP_EVT) {
-        return true;
-    }
-
     return test_bit(bitnr, ov->bitmap) ? true : false;
 }
 
-- 
2.20.1


Re: [Qemu-devel] [PULL 04/60] Revert "spapr: support memory unplug for qtest"
Posted by Greg Kurz 6 years, 11 months ago
On Sun, 10 Mar 2019 19:26:07 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> From: Greg Kurz <groug@kaod.org>
> 
> Commit b8165118f52c broke CPU hotplug tests for old machine types:
> 
> $ QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 ./tests/cpu-plug-test -m=slow
> /ppc64/cpu-plug/pseries-3.1/device-add/2x3x1&maxcpus=6: OK
> /ppc64/cpu-plug/pseries-2.12-sxxm/device-add/2x3x1&maxcpus=6: OK
> /ppc64/cpu-plug/pseries-3.0/device-add/2x3x1&maxcpus=6: OK
> /ppc64/cpu-plug/pseries-2.10/device-add/2x3x1&maxcpus=6: OK
> /ppc64/cpu-plug/pseries-2.11/device-add/2x3x1&maxcpus=6: OK
> /ppc64/cpu-plug/pseries-2.12/device-add/2x3x1&maxcpus=6: OK
> /ppc64/cpu-plug/pseries-2.9/device-add/2x3x1&maxcpus=6: OK
> /ppc64/cpu-plug/pseries-2.7/device-add/2x3x1&maxcpus=6: **
> ERROR:/home/thuth/devel/qemu/hw/ppc/spapr_events.c:313:rtas_event_log_to_source: assertion failed: (source->enabled)
> Broken pipe
> /home/thuth/devel/qemu/tests/libqtest.c:143: kill_qemu() detected QEMU death from signal 6 (Aborted) (core dumped)
> Aborted (core dumped)
> 
> The approach of faking the availability of OV5_HP_EVT causes the
> code to assume the hotplug event source is enabled, which is wrong
> for older machines.
> 
> This reverts commit b8165118f52ce5ee88565d3cec83d30374efdc96.
> 
> A subsequent patch will address the problem of CAS under qtest from
> a different angle.
> 

Since the patches got re-ordered, this sentence is wrong. In case
you re-send this pull request, maybe you can update the changelog
accordingly ?

> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> Message-Id: <155146875097.147873.1732264036668112686.stgit@bahia.lan>
> Tested-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr_ovec.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/hw/ppc/spapr_ovec.c b/hw/ppc/spapr_ovec.c
> index 12510b236a..318bf33de4 100644
> --- a/hw/ppc/spapr_ovec.c
> +++ b/hw/ppc/spapr_ovec.c
> @@ -16,7 +16,6 @@
>  #include "qemu/bitmap.h"
>  #include "exec/address-spaces.h"
>  #include "qemu/error-report.h"
> -#include "sysemu/qtest.h"
>  #include "trace.h"
>  #include <libfdt.h>
>  
> @@ -132,11 +131,6 @@ bool spapr_ovec_test(sPAPROptionVector *ov, long bitnr)
>      g_assert(ov);
>      g_assert(bitnr < OV_MAXBITS);
>  
> -    /* support memory unplug for qtest */
> -    if (qtest_enabled() && bitnr == OV5_HP_EVT) {
> -        return true;
> -    }
> -
>      return test_bit(bitnr, ov->bitmap) ? true : false;
>  }
>  


Re: [Qemu-devel] [PULL 04/60] Revert "spapr: support memory unplug for qtest"
Posted by David Gibson 6 years, 11 months ago
On Mon, Mar 11, 2019 at 11:52:28AM +0100, Greg Kurz wrote:
> On Sun, 10 Mar 2019 19:26:07 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > From: Greg Kurz <groug@kaod.org>
> > 
> > Commit b8165118f52c broke CPU hotplug tests for old machine types:
> > 
> > $ QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 ./tests/cpu-plug-test -m=slow
> > /ppc64/cpu-plug/pseries-3.1/device-add/2x3x1&maxcpus=6: OK
> > /ppc64/cpu-plug/pseries-2.12-sxxm/device-add/2x3x1&maxcpus=6: OK
> > /ppc64/cpu-plug/pseries-3.0/device-add/2x3x1&maxcpus=6: OK
> > /ppc64/cpu-plug/pseries-2.10/device-add/2x3x1&maxcpus=6: OK
> > /ppc64/cpu-plug/pseries-2.11/device-add/2x3x1&maxcpus=6: OK
> > /ppc64/cpu-plug/pseries-2.12/device-add/2x3x1&maxcpus=6: OK
> > /ppc64/cpu-plug/pseries-2.9/device-add/2x3x1&maxcpus=6: OK
> > /ppc64/cpu-plug/pseries-2.7/device-add/2x3x1&maxcpus=6: **
> > ERROR:/home/thuth/devel/qemu/hw/ppc/spapr_events.c:313:rtas_event_log_to_source: assertion failed: (source->enabled)
> > Broken pipe
> > /home/thuth/devel/qemu/tests/libqtest.c:143: kill_qemu() detected QEMU death from signal 6 (Aborted) (core dumped)
> > Aborted (core dumped)
> > 
> > The approach of faking the availability of OV5_HP_EVT causes the
> > code to assume the hotplug event source is enabled, which is wrong
> > for older machines.
> > 
> > This reverts commit b8165118f52ce5ee88565d3cec83d30374efdc96.
> > 
> > A subsequent patch will address the problem of CAS under qtest from
> > a different angle.
> > 
> 
> Since the patches got re-ordered, this sentence is wrong. In case
> you re-send this pull request, maybe you can update the changelog
> accordingly ?

Done.

> 
> > Reported-by: Thomas Huth <thuth@redhat.com>
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > Message-Id: <155146875097.147873.1732264036668112686.stgit@bahia.lan>
> > Tested-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  hw/ppc/spapr_ovec.c | 6 ------
> >  1 file changed, 6 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr_ovec.c b/hw/ppc/spapr_ovec.c
> > index 12510b236a..318bf33de4 100644
> > --- a/hw/ppc/spapr_ovec.c
> > +++ b/hw/ppc/spapr_ovec.c
> > @@ -16,7 +16,6 @@
> >  #include "qemu/bitmap.h"
> >  #include "exec/address-spaces.h"
> >  #include "qemu/error-report.h"
> > -#include "sysemu/qtest.h"
> >  #include "trace.h"
> >  #include <libfdt.h>
> >  
> > @@ -132,11 +131,6 @@ bool spapr_ovec_test(sPAPROptionVector *ov, long bitnr)
> >      g_assert(ov);
> >      g_assert(bitnr < OV_MAXBITS);
> >  
> > -    /* support memory unplug for qtest */
> > -    if (qtest_enabled() && bitnr == OV5_HP_EVT) {
> > -        return true;
> > -    }
> > -
> >      return test_bit(bitnr, ov->bitmap) ? true : false;
> >  }
> >  
> 

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