On Thu, 2 Aug 2018 11:36:19 +0200
Igor Mammedov <imamm...@redhat.com> wrote:
> On Wed, 1 Aug 2018 15:24:30 +0200
> Greg Kurz <gr...@kaod.org> wrote:
>
> > On Tue, 31 Jul 2018 13:25:59 +1000
> > David Gibson <da...@gibson.dropbear.id.au> wrote:
> > [...]
> > > >
> > > > 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
> > > > error.
> > >
> > > 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 agree with what a qtest test should be, but cpu-plug-test doesn't
> > do anything like that obviously, ie, the guest CPU does nothing at
> > all. Only the hotplug path of the QEMU device model that don't need
> > the guest to run is tested here.
> >
> > The more general issue is that paths guarded with kvm_enabled() cannot
> > be tested with a genuine qtest test. That's really unfortunate since
> > these paths are sometimes the one that are mostly used on the field,
> > eg, in-kernel XICS versus emulated XICS.
> >
> > > 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.
> > >
> >
> > If the motivation is to let the test be a true qtest in case someone
> > wants to emulate some guest behavior, I agree the kvm change is wrong.
> >
> > > > 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.
> > >
> >
> > True.
> >
> > > > 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.
> > >
> >
> > So, instead of hijacking the current qtest, we may add a non-qtest test
> > that would start QEMU with "-machine accel=kvm:tcg -S". This would allow
> > at least to test that QEMU doesn't crash right away. And, as suggested
> > by Thomas, the coverage could include SLOF as well if we don't pass -S.
> > But I would need to understand why SLOF sometimes hits a 0x700 when
> > running cpu-plug-test with this patch applied...
> Is cpu-plug-test a qtest one?
> I was under impression it was using tcg accelerator.
>
It starts QEMU with the qtest accelerator, but it doesn't do anything else
to simulate guest behavior. So I don't think it qualifies as a genuine
qtest test.
> we can switch it to accel=kvm:tcg like bios-tables-test did and
> probably reuse boot_sector_init() to make sure that firmware part
> at boot is exercised. Trying to do functional hotplug test on top
> of that (guest side) probably is out of scope of this unit test (too complex)
> and should be left to avocado or likes.
I agree. I'll do that.
Cheers,
--
Greg
>
>
