From: Peter Krempa <pkrempa@redhat.com>
Date: Thu, 13 Oct 2022 12:01:24 +0000 (+0200)
Subject: schema: Re-structure schema for <filesystem> to avoid broken validation
X-Git-Url: http://xenbits.xensource.com/gitweb?a=commitdiff_plain;h=ad5c4be09fe415b4161c4b04ca2ef96f30a0509b;p=libvirt.git

schema: Re-structure schema for <filesystem> to avoid broken validation

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>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---

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"