On 01/16/2017 08:01 PM, Eduardo Habkost wrote: > This is a follow-up to the series that implements > query-cpu-model-expansion. Before including the test script, the > series has some fixes to allow the results of > query-cpu-model-expansion to be used in the QEMU command-line. > > The script probably will work on s390x too, but I couldn't test > it yet. > Eduardo, This test seems to mostly work on s390. The only issue I ran into is querying host model using tcg only. s390 requires kvm to query the host model. Perhaps we could just skip the tcg host test case on s390?

hernejj: ['/usr/local/bin/qemu-system-s390x', '-chardev', 'socket,id=mon,path=/var/tmp/qom-fetch-monitor.sock', '-mon', 'chardev=mon,mode=control', '-display', 'none', '-vga', 'none', '-qtest', 'unix:path=/var/tmp/qom-fetch-qtest.sock', '-qtest-log', '/dev/null', '-machine', 'accel=qtest', '-machine', 'accel=tcg', '-S', '-cpu', 'host'] qemu-system-s390x: CPU definition requires KVM E ====================================================================== ERROR: testTCGModels (__main__.CPUModelTest) ---------------------------------------------------------------------- Traceback (most recent call last): File "./query-cpu-model-test.py", line 380, in testTCGModels self.checkAllCPUModels() File "./query-cpu-model-test.py", line 375, in checkAllCPUModels self.checkOneCPUModel(m) File "./query-cpu-model-test.py", line 304, in checkOneCPUModel self.checkExpansions(model, msg) File "./query-cpu-model-test.py", line 221, in checkExpansions '%s.static' % (msg)) File "./query-cpu-model-test.py", line 177, in checkOneExpansion type=type, model=model['model']) File "./../scripts/qemu.py", line 185, in command raise Exception(reply["error"]["desc"]) Exception: The CPU definition 'host' requires KVM ---------------------------------------------------------------------- Ran 2 tests in 74.622s -- -- Jason J. Herne (jjherne(a)linux.vnet.ibm.com)

On 01/18/2017 12:00 PM, Eduardo Habkost wrote: > On Tue, Jan 17, 2017 at 10:22:10AM -0500, Jason J. Herne wrote: > > On 01/16/2017 08:01 PM, Eduardo Habkost wrote: > > > This is a follow-up to the series that implements > > > query-cpu-model-expansion. Before including the test script, the > > > series has some fixes to allow the results of > > > query-cpu-model-expansion to be used in the QEMU command-line. > > > > > > The script probably will work on s390x too, but I couldn't test > > > it yet. > > > > > > > Eduardo, > > > > This test seems to mostly work on s390. The only issue I ran into is > > querying host model using tcg only. s390 requires kvm to query the host > > model. Perhaps we could just skip the tcg host test case on s390? > > We could still try to test "host", but add it to a greylist where > errors returned by query-cpu-model-expansion can be non-fatal. > query-cpu-model-expansion model="host" can also fail with KVM if > the host doesn't support CPU models. > David had the idea to just support -cpu host for tcg. We could do that. In the meantime, I'm ok with your greylist idea too. This would allow the script to work properly on s390 right from the start, and we can remove the greylist when s390 supports specifying -cpu host for tcg.

Am 18.01.2017 um 18:34 schrieb Eduardo Habkost: > On Wed, Jan 18, 2017 at 12:09:28PM -0500, Jason J. Herne wrote: >> On 01/18/2017 12:00 PM, Eduardo Habkost wrote: >>> On Tue, Jan 17, 2017 at 10:22:10AM -0500, Jason J. Herne wrote: >>>> On 01/16/2017 08:01 PM, Eduardo Habkost wrote: >>>>> This is a follow-up to the series that implements >>>>> query-cpu-model-expansion. Before including the test script, the >>>>> series has some fixes to allow the results of >>>>> query-cpu-model-expansion to be used in the QEMU command-line. >>>>> >>>>> The script probably will work on s390x too, but I couldn't test >>>>> it yet. >>>>> >>>> >>>> Eduardo, >>>> >>>> This test seems to mostly work on s390. The only issue I ran into is >>>> querying host model using tcg only. s390 requires kvm to query the host >>>> model. Perhaps we could just skip the tcg host test case on s390? >>> >>> We could still try to test "host", but add it to a greylist where >>> errors returned by query-cpu-model-expansion can be non-fatal. >>> query-cpu-model-expansion model="host" can also fail with KVM if >>> the host doesn't support CPU models. >>> >> >> David had the idea to just support -cpu host for tcg. We could do that. >> In the meantime, I'm ok with your greylist idea too. This would allow the >> script to work properly on s390 right from the start, and we can remove the >> greylist when s390 supports specifying -cpu host for tcg. > > I believe we will still need to ignore query-cpu-model-expansion > errors on some cases, otherwise the test script will fail on > hosts where KVM doesn't support CPU models in KVM. That is indeed true. For "host" + KVM there would have to be an extra check. non-fatal error sound right for this case (e.g. a warning) > > But we probably don't need a hardcoded greylist, anyway: we could > just make the error non-fatal in case the CPU model is not > reported as migration-safe in query-cpu-definitions. > > But I was wondering: > > 1) Isn't "-cpu host" the default CPU model on s390x on KVM, > even if the host doesn't support CPU models? Yes, it has an inbuilt compatibility mode when specified. If KVM support for cpu models is missing, using "-cpu host" will work (as it worked on QEMU versions without cpu model support), doing something like "-cpu host,vx=on" will not work, as modifying features is not possible (as the interface for query/config is missing). Using "host" for all query-cpu-model is forbiden, as we can't tell what this model looks like. > > 2) Is it really correct to return an error on > "query-cpu-model-expansion model=host type=full" if the host > doesn't support CPU models? > Yes it is, because there is no way to tell which features there are. Returning { name: "host", props: {} } would be misleading, as it would mean that there are no features. Which is wrong. We just can't tell. It is up to the caller to handle this. E.g. ignoring and continuing, doing compatibility stuff or simply reporting an error.

Also think about "query-cpu-model-expansion model=host type=static", which will primarily be used by libvirt on s390x. There is no way to expand this into a static cpu model. Faking anything will just hide errors.

If "host" can't be expanded, QEMU has to be treated like there is no CPU model support (as for older QEMU versions).

> What if it just returned { name: "host", props: {} } > on those cases, meaning that the CPU model is valid and > usable, but QEMU is unable to provide extra information about > it. > -- David

>> Also think about "query-cpu-model-expansion model=host type=static", >> which will primarily be used by libvirt on s390x. There is no way to >> expand this into a static cpu model. Faking anything will just hide errors. > > Yes, static expansion of host model must always return an error > if it's not possible to expand. > >> >> If "host" can't be expanded, QEMU has to be treated like there is no CPU >> model support (as for older QEMU versions). > > OK. I will propose a patch updating the query-cpu-model-expansion > documentation to be more explicit about it. The only real alternative I see would be disabling the query-cpu-model-* interface completely if KVM support is not available. This would however mean, that the same QEMU binary would have the interface when running under TCG, but not when running under KVM on an old KVM version. That also doesn't really feel right, or what do you think?

Am 19.01.2017 um 18:45 schrieb Daniel P. Berrange: > On Thu, Jan 19, 2017 at 06:21:22PM +0100, David Hildenbrand wrote: >> >>>> Also think about "query-cpu-model-expansion model=host type=static", >>>> which will primarily be used by libvirt on s390x. There is no way to >>>> expand this into a static cpu model. Faking anything will just hide errors. >>> >>> Yes, static expansion of host model must always return an error >>> if it's not possible to expand. >>> >>>> >>>> If "host" can't be expanded, QEMU has to be treated like there is no CPU >>>> model support (as for older QEMU versions). >>> >>> OK. I will propose a patch updating the query-cpu-model-expansion >>> documentation to be more explicit about it. >> >> The only real alternative I see would be disabling the query-cpu-model-* >> interface completely if KVM support is not available. >> >> This would however mean, that the same QEMU binary would have the >> interface when running under TCG, but not when running under KVM on an >> old KVM version. >> >> That also doesn't really feel right, or what do you think? > > Yeah that really isn't good. query-cpu-model-* needs to work on TCG > and *not* have a dependancy on KVM in that case, since you can be > running TCG s390 on a x86_64 host, so the host CPU is totally irrelevant > for TCG > Actually what I meant was: TCG: query-cpu-model-* interface always provided KVM (with cpu model support): query-cpu-model-* interface provided KVM (without cpu model support): no query-cpu-model-* interface provided This would avoid having to report an error when expanding "host" in the third case (KVM without cpu model support) but would lead to one QEMU binary having a different set of supported qmp calls when called from TCG and KVM.