1:36 a.m.
*** BLURB HERE ***
Michal Prívozník (2):
capabilityschemadata: Fix order of <features/> in caps-test2.xml
schemas: Drop <interleave/> from capabilities schemas
src/conf/schemas/capability.rng | 160 +++++++++++-----------
src/conf/schemas/domaincaps.rng | 80 +++++------
tests/capabilityschemadata/caps-test2.xml | 8 +-
3 files changed, 119 insertions(+), 129 deletions(-)
--
2.35.1
1:36 a.m.
New subject: [PATCH 1/2] capabilityschemadata: Fix order of <features/> in caps-test2.xml
The guest <features/> is formatted in
virCapabilitiesFormatGuestFeatures() and the order in which
individual child elements are formatted is fixed, because the
function iterates over the virCapsGuestFeatureType enum
(possibly) formatting each member until _LAST is met.
Therefore, <cpuselection/> and <deviceboot/> can't ever be
formatted first. Move these elements to proper location.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tests/capabilityschemadata/caps-test2.xml | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/capabilityschemadata/caps-test2.xml
b/tests/capabilityschemadata/caps-test2.xml
index 125a322998..bf18010440 100644
--- a/tests/capabilityschemadata/caps-test2.xml
+++ b/tests/capabilityschemadata/caps-test2.xml
@@ -72,12 +72,12 @@
</domain>
</arch>
<features>
- <cpuselection/>
- <deviceboot/>
<pae/>
<nonpae/>
<acpi default='on' toggle='yes'/>
<apic default='on' toggle='no'/>
+ <cpuselection/>
+ <deviceboot/>
</features>
</guest>
@@ -114,10 +114,10 @@
</domain>
</arch>
<features>
- <cpuselection/>
- <deviceboot/>
<acpi default='on' toggle='yes'/>
<apic default='on' toggle='no'/>
+ <cpuselection/>
+ <deviceboot/>
</features>
</guest>
--
2.35.1
1:36 a.m.
New subject: [PATCH 2/2] schemas: Drop <interleave/> from capabilities schemas
Currently, we have two types of output only XMLs: capabilities
and domaincapabilities. Neither of these is ever parsed.
Moreover, the order of elements inside these two documents is
well defined by their respective format functions. Therefore,
there's no need to have <interleave/> anywhere in their
corresponding schemas.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/schemas/capability.rng | 160 +++++++++++++++-----------------
src/conf/schemas/domaincaps.rng | 80 ++++++++--------
2 files changed, 115 insertions(+), 125 deletions(-)
diff --git a/src/conf/schemas/capability.rng b/src/conf/schemas/capability.rng
index c7e72c6f39..7a24b2a076 100644
--- a/src/conf/schemas/capability.rng
+++ b/src/conf/schemas/capability.rng
@@ -53,44 +53,40 @@
<define name="secmodel">
<element name="secmodel">
- <interleave>
- <element name="model">
- <text/>
- </element>
- <element name="doi">
- <text/>
- </element>
- <zeroOrMore>
- <element name="baselabel">
- <attribute name="type">
- <text/>
- </attribute>
+ <element name="model">
+ <text/>
+ </element>
+ <element name="doi">
+ <text/>
+ </element>
+ <zeroOrMore>
+ <element name="baselabel">
+ <attribute name="type">
<text/>
- </element>
- </zeroOrMore>
- </interleave>
+ </attribute>
+ <text/>
+ </element>
+ </zeroOrMore>
</element>
</define>
<define name="power_management">
<element name="power_management">
- <interleave>
- <optional>
- <element name="suspend_mem">
- <empty/>
- </element>
- </optional>
- <optional>
- <element name="suspend_disk">
- <empty/>
- </element>
- </optional>
- <optional>
- <element name="suspend_hybrid">
- <empty/>
- </element>
- </optional>
- </interleave>
+ <optional>
+ <element name="suspend_mem">
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <element name="suspend_disk">
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <element name="suspend_hybrid">
+ <empty/>
+ </element>
+ </optional>
</element>
</define>
@@ -442,57 +438,55 @@
<define name="features">
<element name="features">
- <interleave>
- <optional>
- <element name="pae">
- <empty/>
- </element>
- </optional>
- <optional>
- <element name="nonpae">
- <empty/>
- </element>
- </optional>
- <optional>
- <element name="ia64_be">
- <empty/>
- </element>
- </optional>
- <optional>
- <element name="acpi">
- <ref name="featuretoggle"/>
- <empty/>
- </element>
- </optional>
- <optional>
- <element name="apic">
- <ref name="featuretoggle"/>
- <empty/>
- </element>
- </optional>
- <optional>
- <element name="cpuselection">
- <empty/>
- </element>
- </optional>
- <optional>
- <element name="deviceboot">
- <empty/>
- </element>
- </optional>
- <optional>
- <element name="disksnapshot">
- <ref name="featuretoggle"/>
- <empty/>
- </element>
- </optional>
- <optional>
- <element name="hap">
- <ref name="featuretoggle"/>
- <empty/>
- </element>
- </optional>
- </interleave>
+ <optional>
+ <element name="pae">
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <element name="nonpae">
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <element name="ia64_be">
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <element name="acpi">
+ <ref name="featuretoggle"/>
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <element name="apic">
+ <ref name="featuretoggle"/>
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <element name="cpuselection">
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <element name="deviceboot">
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <element name="disksnapshot">
+ <ref name="featuretoggle"/>
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <element name="hap">
+ <ref name="featuretoggle"/>
+ <empty/>
+ </element>
+ </optional>
</element>
</define>
diff --git a/src/conf/schemas/domaincaps.rng b/src/conf/schemas/domaincaps.rng
index 9cbc2467ab..331fdbb1e9 100644
--- a/src/conf/schemas/domaincaps.rng
+++ b/src/conf/schemas/domaincaps.rng
@@ -10,43 +10,41 @@
<define name="domainCapabilities">
<element name="domainCapabilities">
- <interleave>
- <element name="path">
- <ref name="absFilePath"/>
- </element>
- <element name="domain">
- <text/>
- </element>
- <optional>
- <element name="machine">
- <text/>
- </element>
- </optional>
- <element name="arch">
+ <element name="path">
+ <ref name="absFilePath"/>
+ </element>
+ <element name="domain">
+ <text/>
+ </element>
+ <optional>
+ <element name="machine">
<text/>
</element>
- <optional>
- <ref name="vcpu"/>
- </optional>
- <optional>
- <ref name="iothreads"/>
- </optional>
- <optional>
- <ref name="os"/>
- </optional>
- <optional>
- <ref name="cpu"/>
- </optional>
- <optional>
- <ref name="memoryBacking"/>
- </optional>
- <optional>
- <ref name="devices"/>
- </optional>
- <optional>
- <ref name="features"/>
- </optional>
- </interleave>
+ </optional>
+ <element name="arch">
+ <text/>
+ </element>
+ <optional>
+ <ref name="vcpu"/>
+ </optional>
+ <optional>
+ <ref name="iothreads"/>
+ </optional>
+ <optional>
+ <ref name="os"/>
+ </optional>
+ <optional>
+ <ref name="cpu"/>
+ </optional>
+ <optional>
+ <ref name="memoryBacking"/>
+ </optional>
+ <optional>
+ <ref name="devices"/>
+ </optional>
+ <optional>
+ <ref name="features"/>
+ </optional>
</element>
</define>
@@ -78,13 +76,11 @@
<define name="os">
<element name="os">
- <interleave>
- <ref name="supported"/>
- <ref name="enum"/>
- <optional>
- <ref name="loader"/>
- </optional>
- </interleave>
+ <ref name="supported"/>
+ <ref name="enum"/>
+ <optional>
+ <ref name="loader"/>
+ </optional>
</element>
</define>
--
2.35.1
3:42 a.m.
New subject: [PATCH 2/2] schemas: Drop <interleave/> from capabilities schemas
On Wed, Jul 20, 2022 at 08:36:29AM +0200, Michal Privoznik wrote:
Currently, we have two types of output only XMLs: capabilities
and domaincapabilities. Neither of these is ever parsed.
Moreover, the order of elements inside these two documents is
well defined by their respective format functions. Therefore,
there's no need to have <interleave/> anywhere in their
corresponding schemas.
The consumers of libvirt need to parse the XML, and they may wish
to use the RNG files to validate. If the libvirt XML formatter has
ever, or will ever, change ordering of elements, our RNG should
be prepared for that. IOW, we should consider the RNG to be something
that must validate XML output by all previous versions of libvirt.
How confident can we be that we've never changed the order of
any elements ? IMHO its best practice to always use <interleave/>
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/schemas/capability.rng | 160 +++++++++++++++-----------------
src/conf/schemas/domaincaps.rng | 80 ++++++++--------
2 files changed, 115 insertions(+), 125 deletions(-)
diff --git a/src/conf/schemas/capability.rng b/src/conf/schemas/capability.rng
index c7e72c6f39..7a24b2a076 100644
--- a/src/conf/schemas/capability.rng
+++ b/src/conf/schemas/capability.rng
@@ -53,44 +53,40 @@
<define name="secmodel">
<element name="secmodel">
- <interleave>
- <element name="model">
- <text/>
- </element>
- <element name="doi">
- <text/>
- </element>
- <zeroOrMore>
- <element name="baselabel">
- <attribute name="type">
- <text/>
- </attribute>
+ <element name="model">
+ <text/>
+ </element>
+ <element name="doi">
+ <text/>
+ </element>
+ <zeroOrMore>
+ <element name="baselabel">
+ <attribute name="type">
<text/>
- </element>
- </zeroOrMore>
- </interleave>
+ </attribute>
+ <text/>
+ </element>
+ </zeroOrMore>
</element>
</define>
<define name="power_management">
<element name="power_management">
- <interleave>
- <optional>
- <element name="suspend_mem">
- <empty/>
- </element>
- </optional>
- <optional>
- <element name="suspend_disk">
- <empty/>
- </element>
- </optional>
- <optional>
- <element name="suspend_hybrid">
- <empty/>
- </element>
- </optional>
- </interleave>
+ <optional>
+ <element name="suspend_mem">
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <element name="suspend_disk">
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <element name="suspend_hybrid">
+ <empty/>
+ </element>
+ </optional>
</element>
</define>
@@ -442,57 +438,55 @@
<define name="features">
<element name="features">
- <interleave>
- <optional>
- <element name="pae">
- <empty/>
- </element>
- </optional>
- <optional>
- <element name="nonpae">
- <empty/>
- </element>
- </optional>
- <optional>
- <element name="ia64_be">
- <empty/>
- </element>
- </optional>
- <optional>
- <element name="acpi">
- <ref name="featuretoggle"/>
- <empty/>
- </element>
- </optional>
- <optional>
- <element name="apic">
- <ref name="featuretoggle"/>
- <empty/>
- </element>
- </optional>
- <optional>
- <element name="cpuselection">
- <empty/>
- </element>
- </optional>
- <optional>
- <element name="deviceboot">
- <empty/>
- </element>
- </optional>
- <optional>
- <element name="disksnapshot">
- <ref name="featuretoggle"/>
- <empty/>
- </element>
- </optional>
- <optional>
- <element name="hap">
- <ref name="featuretoggle"/>
- <empty/>
- </element>
- </optional>
- </interleave>
+ <optional>
+ <element name="pae">
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <element name="nonpae">
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <element name="ia64_be">
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <element name="acpi">
+ <ref name="featuretoggle"/>
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <element name="apic">
+ <ref name="featuretoggle"/>
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <element name="cpuselection">
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <element name="deviceboot">
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <element name="disksnapshot">
+ <ref name="featuretoggle"/>
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <element name="hap">
+ <ref name="featuretoggle"/>
+ <empty/>
+ </element>
+ </optional>
</element>
</define>
diff --git a/src/conf/schemas/domaincaps.rng b/src/conf/schemas/domaincaps.rng
index 9cbc2467ab..331fdbb1e9 100644
--- a/src/conf/schemas/domaincaps.rng
+++ b/src/conf/schemas/domaincaps.rng
@@ -10,43 +10,41 @@
<define name="domainCapabilities">
<element name="domainCapabilities">
- <interleave>
- <element name="path">
- <ref name="absFilePath"/>
- </element>
- <element name="domain">
- <text/>
- </element>
- <optional>
- <element name="machine">
- <text/>
- </element>
- </optional>
- <element name="arch">
+ <element name="path">
+ <ref name="absFilePath"/>
+ </element>
+ <element name="domain">
+ <text/>
+ </element>
+ <optional>
+ <element name="machine">
<text/>
</element>
- <optional>
- <ref name="vcpu"/>
- </optional>
- <optional>
- <ref name="iothreads"/>
- </optional>
- <optional>
- <ref name="os"/>
- </optional>
- <optional>
- <ref name="cpu"/>
- </optional>
- <optional>
- <ref name="memoryBacking"/>
- </optional>
- <optional>
- <ref name="devices"/>
- </optional>
- <optional>
- <ref name="features"/>
- </optional>
- </interleave>
+ </optional>
+ <element name="arch">
+ <text/>
+ </element>
+ <optional>
+ <ref name="vcpu"/>
+ </optional>
+ <optional>
+ <ref name="iothreads"/>
+ </optional>
+ <optional>
+ <ref name="os"/>
+ </optional>
+ <optional>
+ <ref name="cpu"/>
+ </optional>
+ <optional>
+ <ref name="memoryBacking"/>
+ </optional>
+ <optional>
+ <ref name="devices"/>
+ </optional>
+ <optional>
+ <ref name="features"/>
+ </optional>
</element>
</define>
@@ -78,13 +76,11 @@
<define name="os">
<element name="os">
- <interleave>
- <ref name="supported"/>
- <ref name="enum"/>
- <optional>
- <ref name="loader"/>
- </optional>
- </interleave>
+ <ref name="supported"/>
+ <ref name="enum"/>
+ <optional>
+ <ref name="loader"/>
+ </optional>
</element>
</define>
--
2.35.1
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
5:04 a.m.
New subject: [PATCH 2/2] schemas: Drop <interleave/> from capabilities schemas
On 7/20/22 10:42, Daniel P. Berrangé wrote:
On Wed, Jul 20, 2022 at 08:36:29AM +0200, Michal Privoznik wrote:
> Currently, we have two types of output only XMLs: capabilities
> and domaincapabilities. Neither of these is ever parsed.
> Moreover, the order of elements inside these two documents is
> well defined by their respective format functions. Therefore,
> there's no need to have <interleave/> anywhere in their
> corresponding schemas.
The consumers of libvirt need to parse the XML, and they may wish
to use the RNG files to validate. If the libvirt XML formatter has
ever, or will ever, change ordering of elements, our RNG should
be prepared for that. IOW, we should consider the RNG to be something
that must validate XML output by all previous versions of libvirt.
Well, that's a very strong requirement that we don't even have for
domain XML. I though that using RNG from corresponding libvirt release
is recommended.
How confident can we be that we've never changed the order of
any elements ? IMHO its best practice to always use <interleave/>
We're currently only halfway there. Some elements are allowed to
interleave, some aren't. But okay, let's drop my patches and if we ever
decide to change ordering we'll have domaincapstest, well virschematest
shouting at us.
Michal
860
days inactive
860
days old
4 comments
3 participants
participants (3)
-
Daniel P. Berrangé -
Michal Privoznik -
Michal Prívozník