Bug 1595993

Summary:	Improve the CPU extracting code for (hypervisor-)cpu-baseline/ cpu-compare cmd
Product:	Red Hat Enterprise Linux 7	Reporter:	jiyan <jiyan>
Component:	libvirt	Assignee:	Jiri Denemark <jdenemar>
Status:	CLOSED WONTFIX	QA Contact:	jiyan <jiyan>
Severity:	unspecified	Docs Contact:
Priority:	unspecified
Version:	7.6	CC:	dyuan, dzheng, fjin, jdenemar, jiyan, lcheng, lhuang, lmen, xuzhang, yalzhang
Target Milestone:	rc
Target Release:	---
Hardware:	x86_64
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:
Clones:	1607287 (view as bug list)		Environment:
Last Closed:	2018-11-29 10:20:09 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:
Bug Blocks:	1607287

Description jiyan 2018-06-28 01:50:54 UTC

Description of problem:
Improve the CPU extracting code for (hypervisor-)cpu-baseline/ cpu-compare cmd

Version-Release number of selected component (if applicable):
qemu-kvm-rhev-2.12.0-5.el7.x86_64
libvirt-4.4.0-2.el7.x86_64
kernel-3.10.0-915.el7.x86_64

How reproducible:
100%

Steps to Reproduce:
This bug is derived from the following comment of Bug 1559835
https://bugzilla.redhat.com/show_bug.cgi?id=1559835#c11

(In reply to jiyan from comment #10)
> https://bugzilla.redhat.com/show_bug.cgi?id=1559835#c7
> S3 and S4, the <cpu> definition in S4 is extracted from 'virsh
> domcapabilities' in S3, while the results of 'hypervisor-cpu-compare' are
> different.

S4 in comment #7 does not use a valid CPU definition XML. It should have
failed with an error message similar to S5. Although we should probably change
"<cpu> element" to "CPU definition XML".

> https://bugzilla.redhat.com/show_bug.cgi?id=1559835#c8
> The negative test scenario, error info maybe is a little confusing.

cpu-baseline requires host CPU definition which contains <arch> element within
the <cpu> element. The CPU definition extracted from domcapabilities does not
meet this expectation and thus libvirt fails to parse the CPU. The <arch>
elements you found in cap.xml are outside <cpu> and they are ignored.

The compare and baseline virsh commands (both the old and the new hypervisor
versions) allow domain, capabilities, or domcapabilities XMLs to save users
from having to extract the right CPU definitions manually. The libvirt API
only allows CPU definition XMLs. To )ix these issues (and bug 1592737) we need
to make the CPU extracting code in virsh smarter. Please, file a new bug for
this.

Actual results:


Expected results:


Additional info:

Comment 2 Jiri Denemark 2018-11-22 15:47:32 UTC

Patch sent upstream for review:
https://www.redhat.com/archives/libvir-list/2018-November/msg00838.html

Comment 3 Jiri Denemark 2018-11-23 11:47:21 UTC

Oops, the patch in comment 2 is for a different bug.

Comment 4 Jiri Denemark 2018-11-29 10:20:09 UTC

So I was thinking a bit more about this and I think it is not really a bug we
should fix in any way. The documentation is very clear in both cases:

- the commands accept either a valid CPU definition or a complete domain
  capabilities XML; extracting just the <cpu> element from a domain
  capabilities XML does not produce a valid CPU definition

- cpu-baseline does not mention a CPU from domain capabilities as a supported
  input

That said, a user can pass anything they want to the commands and libvirt will
try to use and parse it as CPU definitions. If the input is not the expected
CPU definition, either error or unexpected result will be reported. It's
similar to passing invalid domain XML; some unknown parts will just be ignored
and we will end up using an XML different from the one used by a user, some of
them will result in an error. With domain XMLs users can ask libvirt to
validate the XML, but I don't think it's practical or even useful to do
something like this for CPU definition XMLs used by the various CPU model
related APIs. Mostly because the XMLs are supposed to be taken either from a
domain XML (which can be validated) or from XMLs produced by libvirt.