Re: [libvirt PATCH v4 1/5] schema: Make element "topology" in host CPU definition optional
On Wed, Oct 07, 2020 at 11:08:21 +0200, Peter Krempa wrote:
On Wed, Oct 07, 2020 at 10:54:54 +0200, Tim Wiederhake wrote:
> This element is not always present, see e.g.
> x86_64-cpuid-Xeon-X5460-host.xml, x86_64-cpuid-Pentium-P6100-host.xml,
> or x86_64-cpuid-EPYC-7601-32-Core-ibpb-host.xml.
Unfortunately this is not enough to persuade me that the change is
correct though.
Jirka?
Right, these are just cputest output files so they can't really be used
as an evidence for optional <topology>.
But it doesn't really matter that much since the host CPU definition is
output only in capabilities XML. APIs that accept host CPU definition as
their input work only on model, vendor, and features thus ignoring any
topology element.
Also looking at the CPU formatting code we format <topology> only if
(def->sockets && def->dies && def->cores &&
def->threads). The question
is whether caps->host.cpu would even be allocated when the topology is
unknown. In any case, I think it's fine to mark it as optional in the
schema. Another option would be to mock some topology in cputest to make
sure the host CPU definitions contain the corresponding element.
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>