src/conf/schemas/domaincommon.rng | 365 +++++++++++++++--------------- 1 file changed, 186 insertions(+), 179 deletions(-)
The validation of a '<filesystem type='mount'>' device fails if the
elements inside are not ordered in the order in the schema despite using
<interleave>. This is a bug in libxml2's validator as removing the
'<optional>' property from the definition of the 'type' attribute with
'mount' variable fixes the problem.
I've reported it as another instance of a seemingly related issue:
https://gitlab.gnome.org/GNOME/libxml2/-/issues/131
Meanwhile libvirt can re-arrange the schema by extracting the common
bits into a new definition and referencing them from each of the choice
groups explicitly.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/392
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
src/conf/schemas/domaincommon.rng | 365 +++++++++++++++---------------
1 file changed, 186 insertions(+), 179 deletions(-)
diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
index 5ff15e8787..d346442510 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -2860,205 +2860,212 @@
</interleave>
</element>
</define>
+
+ <define name="filesystemCommon">
+ <interleave>
+ <element name="target">
+ <attribute name="dir"/>
+ <empty/>
+ </element>
+ <optional>
+ <attribute name="accessmode">
+ <choice>
+ <value>passthrough</value>
+ <value>mapped</value>
+ <value>squash</value>
+ </choice>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="multidevs">
+ <choice>
+ <value>default</value>
+ <value>remap</value>
+ <value>forbid</value>
+ <value>warn</value>
+ </choice>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="fmode">
+ <ref name="createMode"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="dmode">
+ <ref name="createMode"/>
+ </attribute>
+ </optional>
+ <optional>
+ <element name="readonly">
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <ref name="deviceBoot"/>
+ </optional>
+ <optional>
+ <ref name="alias"/>
+ </optional>
+ <optional>
+ <ref name="acpi"/>
+ </optional>
+ <optional>
+ <ref name="address"/>
+ </optional>
+ <optional>
+ <element name="space_hard_limit">
+ <ref name="scaledInteger"/>
+ </element>
+ </optional>
+ <optional>
+ <element name="space_soft_limit">
+ <ref name="scaledInteger"/>
+ </element>
+ </optional>
+ </interleave>
+ </define>
+
<define name="filesystem">
<element name="filesystem">
- <interleave>
- <choice>
- <group>
- <attribute name="type">
- <value>file</value>
- </attribute>
- <interleave>
- <optional>
- <ref name="fsDriver"/>
- </optional>
- <element name="source">
- <attribute name="file">
- <ref name="absFilePath"/>
- </attribute>
- <empty/>
- </element>
- </interleave>
- </group>
- <group>
- <attribute name="type">
- <value>block</value>
- </attribute>
- <interleave>
- <optional>
- <ref name="fsDriver"/>
- </optional>
- <element name="source">
- <attribute name="dev">
- <ref name="absFilePath"/>
- </attribute>
- <empty/>
- </element>
- </interleave>
- </group>
- <group>
- <!-- type="mount" is default -->
+ <optional>
+ <attribute name="model">
+ <choice>
+ <value>virtio</value>
+ <value>virtio-transitional</value>
+ <value>virtio-non-transitional</value>
+ </choice>
+ </attribute>
+ </optional>
+ <choice>
+ <group>
+ <attribute name="type">
+ <value>file</value>
+ </attribute>
+ <interleave>
<optional>
- <attribute name="type">
- <value>mount</value>
- </attribute>
+ <ref name="fsDriver"/>
</optional>
- <interleave>
- <optional>
- <ref name="fsDriver"/>
- </optional>
- <optional>
- <ref name="fsBinary"/>
- </optional>
- <element name="source">
- <choice>
- <group>
- <attribute name="dir">
- <ref name="absDirPath"/>
- </attribute>
- </group>
- <group>
- <attribute name="socket">
- <ref name="absFilePath"/>
- </attribute>
- </group>
- </choice>
- <empty/>
- </element>
- </interleave>
- </group>
- <group>
- <optional>
- <attribute name="type">
- <value>bind</value>
+ <element name="source">
+ <attribute name="file">
+ <ref name="absFilePath"/>
</attribute>
+ <empty/>
+ </element>
+ <ref name="filesystemCommon"/>
+ </interleave>
+ </group>
+ <group>
+ <attribute name="type">
+ <value>block</value>
+ </attribute>
+ <interleave>
+ <optional>
+ <ref name="fsDriver"/>
</optional>
- <interleave>
- <optional>
- <ref name="fsDriver"/>
- </optional>
- <element name="source">
- <attribute name="dir">
- <ref name="absDirPath"/>
- </attribute>
- <empty/>
- </element>
- </interleave>
- </group>
- <group>
- <attribute name="type">
- <value>template</value>
- </attribute>
- <interleave>
- <optional>
- <ref name="fsDriver"/>
- </optional>
- <element name="source">
- <attribute name="name">
- <ref name="genericName"/>
- </attribute>
- <empty/>
- </element>
- </interleave>
- </group>
- <group>
- <attribute name="type">
- <value>ram</value>
- </attribute>
- <interleave>
- <optional>
- <ref name="fsDriver"/>
- </optional>
- <element name="source">
- <attribute name="usage">
- <ref name="unsignedLong"/>
- </attribute>
- <optional>
- <attribute name="units">
- <ref name="unit"/>
- </attribute>
- </optional>
- <empty/>
- </element>
- </interleave>
- </group>
- </choice>
- <interleave>
- <element name="target">
- <attribute name="dir"/>
- <empty/>
- </element>
+ <element name="source">
+ <attribute name="dev">
+ <ref name="absFilePath"/>
+ </attribute>
+ <empty/>
+ </element>
+ <ref name="filesystemCommon"/>
+ </interleave>
+ </group>
+ <group>
+ <!-- type="mount" is default -->
<optional>
- <attribute name="accessmode">
- <choice>
- <value>passthrough</value>
- <value>mapped</value>
- <value>squash</value>
- </choice>
+ <attribute name="type">
+ <value>mount</value>
</attribute>
</optional>
- <optional>
- <attribute name="multidevs">
+ <interleave>
+ <optional>
+ <ref name="fsDriver"/>
+ </optional>
+ <optional>
+ <ref name="fsBinary"/>
+ </optional>
+ <element name="source">
<choice>
- <value>default</value>
- <value>remap</value>
- <value>forbid</value>
- <value>warn</value>
+ <group>
+ <attribute name="dir">
+ <ref name="absDirPath"/>
+ </attribute>
+ </group>
+ <group>
+ <attribute name="socket">
+ <ref name="absFilePath"/>
+ </attribute>
+ </group>
</choice>
- </attribute>
- </optional>
- <optional>
- <attribute name="fmode">
- <ref name="createMode"/>
- </attribute>
- </optional>
- <optional>
- <attribute name="dmode">
- <ref name="createMode"/>
- </attribute>
- </optional>
- <optional>
- <element name="readonly">
<empty/>
</element>
- </optional>
- <optional>
- <ref name="deviceBoot"/>
- </optional>
- <optional>
- <ref name="alias"/>
- </optional>
- <optional>
- <ref name="acpi"/>
- </optional>
+ <ref name="filesystemCommon"/>
+ </interleave>
+ </group>
+ <group>
<optional>
- <ref name="address"/>
+ <attribute name="type">
+ <value>bind</value>
+ </attribute>
</optional>
- </interleave>
- <interleave>
- <optional>
- <element name="space_hard_limit">
- <ref name="scaledInteger"/>
+ <interleave>
+ <optional>
+ <ref name="fsDriver"/>
+ </optional>
+ <element name="source">
+ <attribute name="dir">
+ <ref name="absDirPath"/>
+ </attribute>
+ <empty/>
</element>
- </optional>
- <optional>
- <element name="space_soft_limit">
- <ref name="scaledInteger"/>
+ <ref name="filesystemCommon"/>
+ </interleave>
+ </group>
+ <group>
+ <attribute name="type">
+ <value>template</value>
+ </attribute>
+ <interleave>
+ <optional>
+ <ref name="fsDriver"/>
+ </optional>
+ <element name="source">
+ <attribute name="name">
+ <ref name="genericName"/>
+ </attribute>
+ <empty/>
</element>
- </optional>
- </interleave>
- <optional>
- <attribute name="model">
- <choice>
- <value>virtio</value>
- <value>virtio-transitional</value>
- <value>virtio-non-transitional</value>
- </choice>
+ <ref name="filesystemCommon"/>
+ </interleave>
+ </group>
+ <group>
+ <attribute name="type">
+ <value>ram</value>
</attribute>
- </optional>
- </interleave>
+ <interleave>
+ <optional>
+ <ref name="fsDriver"/>
+ </optional>
+ <element name="source">
+ <attribute name="usage">
+ <ref name="unsignedLong"/>
+ </attribute>
+ <optional>
+ <attribute name="units">
+ <ref name="unit"/>
+ </attribute>
+ </optional>
+ <empty/>
+ </element>
+ <ref name="filesystemCommon"/>
+ </interleave>
+ </group>
+ </choice>
</element>
</define>
+
<define name="fsDriver">
<element name="driver">
<!-- Annoying inconsistency. "disk" uses "name"
--
2.37.3On Thu, Oct 13, 2022 at 02:57:33PM +0200, Peter Krempa wrote:
> The validation of a '<filesystem type='mount'>' device fails if the
> elements inside are not ordered in the order in the schema despite using
> <interleave>. This is a bug in libxml2's validator as removing the
> '<optional>' property from the definition of the 'type' attribute with
> 'mount' variable fixes the problem.
>
> I've reported it as another instance of a seemingly related issue:
>
> https://gitlab.gnome.org/GNOME/libxml2/-/issues/131
>
> Meanwhile libvirt can re-arrange the schema by extracting the common
> bits into a new definition and referencing them from each of the choice
> groups explicitly.
>
> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/392
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
> src/conf/schemas/domaincommon.rng | 365 +++++++++++++++---------------
> 1 file changed, 186 insertions(+), 179 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
We've had many of these schema ordering problems, and I wondered if
we've got them all yet. To the surprise of absolutely no one, the
answer is no.
If we take the view that *every* single element should be re-orderable,
which is reasonable as I've seen mgmt apps generate libvirt XML in
totally arbitrary ordering, then this dumb script reverses everything
we have in our samples:
$ cat tests/xmlrngfuzz.py
#!/usr/bin/python
from lxml import etree
import sys
tree = etree.parse(sys.stdin)
root = tree.getroot()
def reverse(node):
newe = []
for e in list(node):
node.remove(e)
newe.append(e)
newe.reverse()
for e in newe:
node.append(e)
pass
for e in newe:
reverse(e)
reverse(root)
print(etree.tostring(tree, pretty_print=True).decode('utf8'))
$ for i in `find -name '*.xml' `
do
echo $1
./xmlrngfuzz.py < $i > ${i%%xml}rev.xml
done
$ ./build/tests/virschematest
...
Some tests failed. Run them using:
VIR_TEST_DEBUG=1 VIR_TEST_RANGE=2-3,7-8,11-15,17,19,21,26-27,29,32,34,39,41-44,51,54,58,62,71,93,108,139,206,221,306,309,357,369,410,470,504,513,529,568-569,590,648,651,660,678,719,741,771-772,775,854,883,921-922,933,1059,1150,1171,1177,1218,1254,1268,1288,1338,1345,1386,1398,1500,1541,1573,1649,1663,1674-1675,1686,1735,1760,1779,1782,1787,1791,1814,1896,1971,1980,2015,2056,2077,2114,2146,2174,2188,2209-2210,2214-2215,2217-2218,2231,2237,2240,2245,2258,2265,2267,2274,2278,2292,2299,2304,2306,2328,2343,2369,2374,2383,2397,2420,2424,2432,2451,2457-2458,2465,2477,2498,2517,2522,2533,2539,2542,2544,2546,2563,2584,2605,2615,2623-2624,2632,2637,2645,2653,2655,2681,2686,2700,2706,2724,2734,2747,2750,2753,2767,2791,2804,2810,2835-2836,2838,2841,2844,2858,2865,2876,2906,2934,2940,2942,2950,2959,2977,2985-2986,2988,2999,3019-3020,3033,3051,3054-3055,3065,3088,3114,3117,3122,3130-3131,3134,3144,3167,3171,3188,3213,3216,3222,3232,3236,3250,3275,3290,3292,3302,3307,3355,3370,3387,3400,3420,3427,3429,3436,3453,3467,3477,3486,3517,3519-3521,3523-3525,3530,3533-3536,3539,3543,3550,3578,3600,3649,3652,3657,3664,3668,3675,3693,3700,3704,3708,3743,3748,3768,3805,3808,3814,3885,3891,3913,3925-3926,3938,3947,3949,3960-3961,3964-3965,3998,4001,4004,4014,4031,4036,4048,4050,4056,4088,4093,4104,4166,4169-4170,4172-4173,4179,4181,4183,4185,4188-4191,4234,4240,4401,4477,4544,4550,4559-4562,4566-4567,4569,4571-4573,4576-4577,4579-4581,4583-4585,4587-4589,4592,4594,4597,4601-4602,4604-4605,4608-4610,4615,4621,4624,4626,4628,4633,4635,4639,4641,4643,4645-4646,4648-4651,4653-4654,4657,4659-4660,4663-4664,4679,4695,4700,4706,4715,4719,4739,4742,5107,5111,5113-5117,5121-5122,5125-5126,5130,5132-5133,5135,5137-5139,5145-5146,5149,5153-5155,5157,5163-5165,5167,5169-5173,5179-5180,5184,5187-5189,5191-5194,5197-5201,5203,5206-5207,5212-5213,5215-5216,5219,5222,5225,5230,5234,5236-5239,5242,5246,5248-5251,5253-5256,5260,5262,5264-5265,5267,5269-5272,5281-5284,5286-5288,5291,5293,5295,5298,5300-5302,5328-5330,5332,5335,5340,5343-5344,5347,5349-5350,5353-5357,5359,5362-5365,5368,5371,5373-5374,5376,5378-5381,5385,5387,5394-5395,5397,5401-5402,5404,5406-5408,5410,5412,5414,5416-5418,5421-5423,5427-5429,5432,5434-5436,5438-5440,5444-5446,5448-5451,5455-5456,5463,5466-5467,5469,5472,5475,5477-5478,5480-5481,5487-5488,5491,5493-5494,5513,5515,5529,5595,5607,5671,5673,5730,5772,5972,5981,5985,6004,6020,6022-6023,6045,6057,6078,6103,6105,6116,6124,6135 ./build/tests/virschematest
Some work todo....
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 :|
On Thu, Oct 13, 2022 at 14:25:30 +0100, Daniel P. Berrangé wrote:
> On Thu, Oct 13, 2022 at 02:57:33PM +0200, Peter Krempa wrote:
> > The validation of a '<filesystem type='mount'>' device fails if the
> > elements inside are not ordered in the order in the schema despite using
> > <interleave>. This is a bug in libxml2's validator as removing the
> > '<optional>' property from the definition of the 'type' attribute with
> > 'mount' variable fixes the problem.
> >
> > I've reported it as another instance of a seemingly related issue:
> >
> > https://gitlab.gnome.org/GNOME/libxml2/-/issues/131
> >
> > Meanwhile libvirt can re-arrange the schema by extracting the common
> > bits into a new definition and referencing them from each of the choice
> > groups explicitly.
> >
> > Resolves: https://gitlab.com/libvirt/libvirt/-/issues/392
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> > src/conf/schemas/domaincommon.rng | 365 +++++++++++++++---------------
> > 1 file changed, 186 insertions(+), 179 deletions(-)
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
>
> We've had many of these schema ordering problems, and I wondered if
> we've got them all yet. To the surprise of absolutely no one, the
> answer is no.
I'm not surprised about that. I was surprised though about what this
patch fixes as the schema is actually correct in this case.
[...]
> reverse(root)
> print(etree.tostring(tree, pretty_print=True).decode('utf8'))
>
>
> $ for i in `find -name '*.xml' `
> do
> echo $1
> ./xmlrngfuzz.py < $i > ${i%%xml}rev.xml
> done
This also includes stuff like:
- '*-invalid.xml' files
- these are excluded from validation as they are intentionally
invalid, but no longer match exclusion pattern after reversal
- output-only XML such as capabilities/domcapabilities
- we are strictly defining the order there, I'm not sure if it makes
sense to allow interleaving
Disregarding the above it's not that bad. I've seen ~40 instances e.g.
in qemuxml2argvdata boiling down to 7 actual mistakes in the scheama.
I'll have a look at some more input xmls, but I don't feel like we
should touch the schema for output-only ones.
On a Thursday in 2022, Peter Krempa wrote: >The validation of a '<filesystem type='mount'>' device fails if the >elements inside are not ordered in the order in the schema despite using ><interleave>. This is a bug in libxml2's validator as removing the >'<optional>' property from the definition of the 'type' attribute with >'mount' variable fixes the problem. > >I've reported it as another instance of a seemingly related issue: > > https://gitlab.gnome.org/GNOME/libxml2/-/issues/131 > Fun! >Meanwhile libvirt can re-arrange the schema by extracting the common >bits into a new definition and referencing them from each of the choice >groups explicitly. > >Resolves: https://gitlab.com/libvirt/libvirt/-/issues/392 >Signed-off-by: Peter Krempa <pkrempa@redhat.com> >--- > src/conf/schemas/domaincommon.rng | 365 +++++++++++++++--------------- > 1 file changed, 186 insertions(+), 179 deletions(-) > Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
© 2016 - 2024 Red Hat, Inc.