Re: [libvirt] [PATCHv2 5/9] conf: split <disk> schema into more pieces
On Mon, Apr 14, 2014 at 16:54:16 -0600, Eric Blake wrote:
Disk snapshots use a subset of <disk> sources (no directory or
pool support yet, and while domain disks support a startupPolicy,
it doesn't make sense for snapshots). This patch lets the two
RelaxNG grammars share a bit more code, as well as factoring
things into pieces that will be easier to move to a common file
for sharing with storage volumes. It relies on the ability
to override definitions as part of an include, set up in the
previous patch.
The diff is a bit hard to read, because it mixes reindentation
with refactoring; 'git diff -b --patience' may help.
* docs/schemas/domaincommon.rng (disk): Refactor into pieces.
(diskSource, diskSourceFile, diskSourceBlock, diskSourceDir)
(diskSourceVolume: New defines.
(diskSourceNetwork): Revise scope.
* docs/schemas/domainsnapshot.rng (disksnapshot): Adjust.
(disksnapshotsource): New define.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
docs/schemas/domaincommon.rng | 329 +++++++++++++++++++++-------------------
docs/schemas/domainsnapshot.rng | 63 ++------
2 files changed, 179 insertions(+), 213 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index df2c839..7739fd8 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
...
diff --git a/docs/schemas/domainsnapshot.rng
b/docs/schemas/domainsnapshot.rng
index bec12db..ef3135f 100644
--- a/docs/schemas/domainsnapshot.rng
+++ b/docs/schemas/domainsnapshot.rng
@@ -133,65 +133,20 @@
<value>external</value>
</attribute>
</optional>
- <choice>
- <group>
- <optional>
- <attribute name='type'>
- <value>file</value>
- </attribute>
- </optional>
- <interleave>
- <optional>
- <element name='source'>
- <optional>
- <attribute name='file'>
- <ref name='absFilePath'/>
- </attribute>
- </optional>
- <optional>
- <ref name='storageStartupPolicy'/>
- </optional>
- <empty/>
- </element>
- </optional>
- <ref name='storageSourceExtra'/>
- </interleave>
- </group>
...
- </choice>
+ <ref name="disksnapshotsource"/>
</group>
</choice>
</element>
</define>
+ <define name='disksnapshotsource'>
+ <choice>
+ <ref name='diskSourceFile'/>
+ <ref name='diskSourceBlock'/>
+ <ref name='diskSourceNetwork'/>
+ </choice>
+ </define>
This would allow <ref name='devSeclabel'/> in
/domainsnapshot/disks/disk/source even though it was not allowed before.
For example, the attached XML would be considered valid after this
patch. And I see the follow-up patches add more stuff that is allowed in
domain snapshot XML however there's no code that would actually use the
new stuff, is it? I think we should take the changes in domaincommon.rng
while leaving domainsnapshot.rng alone. You started playing with making
some stuff usable only in domain XML and another stuff only in
domain snapshot XML but unfortunately there's a lot of other elements
that would need similar treatment. Making both XMLs reuse the same
source schema is a great idea but it's going to be a time-consuming task
and the possibility for the result to become even less readable than two
distinct definitions is very high.
ACK to the domaincommon.rng part.
Jirka
diff --git c/tests/domainsnapshotxml2xmlin/disk-seclabel-invalid.xml
i/tests/domainsnapshotxml2xmlin/disk-seclabel-invalid.xml
new file mode 100644
index 0000000..528c646
--- /dev/null
+++ i/tests/domainsnapshotxml2xmlin/disk-seclabel-invalid.xml
@@ -0,0 +1,11 @@
+<domainsnapshot>
+ <name>my snap name</name>
+ <description>!@#$%^</description>
+ <disks>
+ <disk name='hde' snapshot='external' type='file'>
+ <source file='/path/to/new2'>
+ <seclabel model='dac' relabel='no'/>
+ </source>
+ </disk>
+ </disks>
+</domainsnapshot>