Re: [Qemu-devel] [PATCH for-3.1] tests/cpu-plug-test: check CPU hotplug on ppc64 with KVM

[Greg Kurz]

On Fri, 27 Jul 2018 15:27:24 +1000
David Gibson <address@hidden> wrote:

> On Wed, Jul 25, 2018 at 04:45:26PM +0200, Greg Kurz wrote:
> > Commit b585395b655 fixed a regression introduced by some recent changes
> > in the XICS code, that was causing QEMU to crash instantly during CPU
> > hotplug with KVM. This is typically the kind of bug we'd like our
> > test suite to detect before it gets merged. Unfortunately, the current
> > tests run with '-machine accel=qtest' and don't exercise KVM specific
> > paths in QEMU.
> > 
> > This patch hence changes add_pseries_test_case() to launch QEMU with
> > '-machine accel=kvm' if KVM is available.
> > 
> > A notable consequence is that the guest will execute SLOF, but for some
> > reasons SLOF sometimes hits a program exception. This causes the guest
> > to loop forever and the test to be stuck.  Since we don't need the guest
> > to be truely running, let's pass -S to QEMU to avoid that.
> > 
> > Also disable machine capabilities that could be unavailable in KVM, eg,
> > when using PR KVM.
> > 
> > Signed-off-by: Greg Kurz <address@hidden>  
> 
> I'm pretty sure trying to change the accelerator on a qtest test just
> doesn't make sense.  We'd need a different approach for testing cpu
> hotplug against kvm & tcg backends.
> 

The test starts QEMU, triggers the CPU hotplug code with a QMP command
and checks the command didn't fail (or QEMU didn't crash, as it would
have before commit b585395b655a)... I really don't understand what
is wrong with that... Please elaborate.

> > ---
> >  tests/cpu-plug-test.c |   20 ++++++++++++++++----
> >  1 file changed, 16 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tests/cpu-plug-test.c b/tests/cpu-plug-test.c
> > index 5f39ba0df394..2107557809b7 100644
> > --- a/tests/cpu-plug-test.c
> > +++ b/tests/cpu-plug-test.c
> > @@ -21,6 +21,7 @@ struct PlugTestData {
> >      unsigned cores;
> >      unsigned threads;
> >      unsigned maxcpus;
> > +    const char *extra_args;
> >  };
> >  typedef struct PlugTestData PlugTestData;
> >  
> > @@ -106,9 +107,10 @@ static void 
> > test_plug_with_device_add_coreid(gconstpointer data)
> >      char *args;
> >      unsigned int c;
> >  
> > -    args = g_strdup_printf("-machine %s -cpu %s "
> > +    args = g_strdup_printf("-machine %s -cpu %s %s "
> >                             "-smp 
> > 1,sockets=%u,cores=%u,threads=%u,maxcpus=%u",
> >                             td->machine, td->cpu_model,
> > +                           td->extra_args ? td->extra_args : "",
> >                             td->sockets, td->cores, td->threads, 
> > td->maxcpus);
> >      qtest_start(args);
> >  
> > @@ -195,10 +197,20 @@ static void add_pseries_test_case(const char *mname)
> >          (g_str_has_prefix(mname, "pseries-2.") && atoi(&mname[10]) < 7)) {
> >          return;
> >      }
> > -    data = g_new(PlugTestData, 1);
> > +    data = g_new0(PlugTestData, 1);
> >      data->machine = g_strdup(mname);
> >      data->cpu_model = "power8_v2.0";
> > -    data->device_model = g_strdup("power8_v2.0-spapr-cpu-core");
> > +    if (!access("/sys/module/kvm_hv", F_OK) ||
> > +        !access("/sys/module/kvm_pr", F_OK)) {
> > +        data->cpu_model = "host";
> > +        data->extra_args =
> > +            "-machine accel=kvm "
> > +            "-machine cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken "
> > +            "-machine cap-htm=off "
> > +            "-machine cap-hpt-max-page-size=64k "
> > +            "-S";
> > +    }
> > +    data->device_model = g_strdup_printf("%s-spapr-cpu-core", 
> > data->cpu_model);
> >      data->sockets = 2;
> >      data->cores = 3;
> >      data->threads = 1;
> > @@ -221,7 +233,7 @@ static void add_s390x_test_case(const char *mname)
> >          return;
> >      }
> >  
> > -    data = g_new(PlugTestData, 1);
> > +    data = g_new0(PlugTestData, 1);
> >      data->machine = g_strdup(mname);
> >      data->cpu_model = "qemu";
> >      data->device_model = g_strdup("qemu-s390x-cpu");
> >   
>

pgpbuxGaTP2Z7.pgp
Description: OpenPGP digital signature