From: David Gibson <email@example.com>
To: Greg Kurz <firstname.lastname@example.org>
Cc: email@example.com, "Thomas Huth" <firstname.lastname@example.org>,
"Cédric Le Goater" <email@example.com>
Subject: Re: [Qemu-devel] [PATCH for-3.1] tests/cpu-plug-test: check CPU hotplug on ppc64 with KVM
Date: Tue, 31 Jul 2018 13:25:59 +1000 [thread overview]
Message-ID: <20180731032559.GH2708@umbus.fritz.box> (raw)
[-- Attachment #1: Type: text/plain, Size: 3955 bytes --]
On Mon, Jul 30, 2018 at 10:41:45AM +0200, Greg Kurz wrote:
> On Mon, 30 Jul 2018 15:57:15 +1000
> David Gibson <firstname.lastname@example.org> wrote:
> > On Fri, Jul 27, 2018 at 09:54:52AM +0200, Greg Kurz wrote:
> > > On Fri, 27 Jul 2018 15:27:24 +1000
> > > David Gibson <email@example.com> 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 <firstname.lastname@example.org>
> > > >
> > > > 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.
> > Well, ok, let me turn that around. A test that doesn't rely on
> > controlling the guest side behaviour at all probably shouldn't be a
> > qtest based test, since that's what qtest is all about.
> The CPU hotplug test doesn't seem to do anything on the guest side: it
> just checks that 'device_add' returns a response that isn't an
Right, which makes this ill suited to being a qtest test. The whole
point of qtest is making it easier to test qemu peripherals without
having to have specific test code within the guest, by essentially
replacing the guest's cpu with a stub controlled by the test harness.
That's what the qtest accel is all about.
I think it's confusing to have a test which tries things with both
qtest and kvm accelerators. Looking like a qtest test, people might
reasonably think they can extend/refine the test to check behaviour
when the guest does respond to the hotplug events. But such an
extension won't work with the kvm accel, because the qtest code used
to simulate that guest response won't have any effect with kvm.
> I'm not aware that the guest is expected to have a specific behavior
> during 'device_add', apart from not crashing or hanging. That was the
> initial idea behind passing '-S' to ensure the guest doesn't run.
Note that with qtest (or -S) we don't even test that minimal
condition. We only test that *qemu* doesn't crash - it could fatally
compromise the guest and the test would never know.
> Your remark seems to be more general though... are you meaning that
> doing something like qtest_start("-machine accel=kvm:tcg") is just
> wrong ?
Pretty much, yes. A non-qtest test which does that is reasonable, but
not a qtest test.
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_!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2018-07-31 3:28 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-25 14:45 [Qemu-devel] [PATCH for-3.1] tests/cpu-plug-test: check CPU hotplug on ppc64 with KVM Greg Kurz
2018-07-27 5:27 ` David Gibson
2018-07-27 7:54 ` Greg Kurz
2018-07-27 8:18 ` Thomas Huth
2018-07-27 9:00 ` Greg Kurz
2018-07-27 11:25 ` Thomas Huth
2018-07-27 12:03 ` Greg Kurz
2018-07-30 5:57 ` David Gibson
2018-07-30 8:41 ` Greg Kurz
2018-07-30 9:59 ` Greg Kurz
2018-07-31 3:27 ` David Gibson
2018-08-01 13:35 ` Greg Kurz
2018-07-31 3:25 ` David Gibson [this message]
2018-08-01 13:24 ` Greg Kurz
2018-08-02 4:08 ` David Gibson
2018-08-02 9:36 ` Igor Mammedov
2018-08-02 10:52 ` Greg Kurz
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.