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.

Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
---
 docs/schemas/cputypes.rng | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/docs/schemas/cputypes.rng b/docs/schemas/cputypes.rng
index 88f6904343..aea6ff0267 100644
--- a/docs/schemas/cputypes.rng
+++ b/docs/schemas/cputypes.rng
@@ -336,17 +336,19 @@
             </attribute>
           </element>
         </optional>
-        <element name="topology">
-          <attribute name="sockets">
-            <ref name="positiveInteger"/>
-          </attribute>
-          <attribute name="cores">
-            <ref name="positiveInteger"/>
-          </attribute>
-          <attribute name="threads">
-            <ref name="positiveInteger"/>
-          </attribute>
-        </element>
+        <optional>
+          <element name="topology">
+            <attribute name="sockets">
+              <ref name="positiveInteger"/>
+            </attribute>
+            <attribute name="cores">
+              <ref name="positiveInteger"/>
+            </attribute>
+            <attribute name="threads">
+              <ref name="positiveInteger"/>
+            </attribute>
+          </element>
+        </optional>
         <zeroOrMore>
           <element name="feature">
             <attribute name="name">
-- 
2.26.2

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?

> 
> Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
> ---

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@redhat.com>

On Wed, Oct 07, 2020 at 12:29:42 +0200, Jiri Denemark wrote:
> 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@redhat.com>

Pushed now.

Jirka