[libvirt] [PATCH 0/9] i386: query-cpu-model-expansion test script

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. This series and its dependencies can be pulled from the branch: https://github.com/ehabkost/qemu-hacks.git work/x86-query-cpu-expansion-test --- Cc: Cornelia Huck <cornelia.huck@de.ibm.com> Cc: Christian Borntraeger <borntraeger@de.ibm.com> Cc: David Hildenbrand <david@redhat.com> Cc: libvir-list@redhat.com Cc: Jiri Denemark <jdenemar@redhat.com> Cc: "Jason J. Herne" <jjherne@linux.vnet.ibm.com> Cc: Markus Armbruster <armbru@redhat.com> Cc: Richard Henderson <rth@twiddle.net> Cc: Igor Mammedov <imammedo@redhat.com> Cc: Eric Blake <eblake@redhat.com> Eduardo Habkost (9): target-i386: Move "host" properties to base class target-i386: Allow short strings to be used as vendor ID cpu: Support comma escaping when parsing -cpu qemu.py: Make logging optional qtest.py: Support QTEST_LOG environment variable qtest.py: make logging optional qtest.py: Make 'binary' parameter optional tests: Add rules to non-gtester qtest test cases tests: Test case for query-cpu-model-expansion scripts/qemu.py | 25 ++- scripts/qtest.py | 15 +- qom/cpu.c | 32 ++-- target/i386/cpu.c | 83 ++++----- tests/test-x86-cpuid-compat.c | 19 ++ tests/Makefile.include | 40 ++++- tests/query-cpu-model-test.py | 398 ++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 551 insertions(+), 61 deletions(-) create mode 100755 tests/query-cpu-model-test.py -- 2.11.0.259.g40922b1

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@linux.vnet.ibm.com)

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.
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@linux.vnet.ibm.com)
-- Eduardo

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. -- -- Jason J. Herne (jjherne@linux.vnet.ibm.com)

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. 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? 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? 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. -- Eduardo

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

On Wed, Jan 18, 2017 at 08:18:40PM +0100, David Hildenbrand wrote:
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.
Well, if there's a risk { props: {} } will be interpreted as "all features disabled" instead of "we don't know the real value for any feature", then I agree that returning an error is better and safer.
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.
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
-- Eduardo

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

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 Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|

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.
Regards, Daniel
-- David

On Fri, Jan 20, 2017 at 03:30:54PM +0100, David Hildenbrand wrote:
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.
I don't think we should do that. Expansion of static and migration-safe CPU models is host-independent by design, so it should always work on any host. Management software could even choose to run CPU expansion on one host, and start the VM (using the results of the expansion) on a different host. -- Eduardo
participants (4)
-
Daniel P. Berrange
-
David Hildenbrand
-
Eduardo Habkost
-
Jason J. Herne