In general, we try to make virt-xml-validate tolerant of input elements in any order when possible. However, as written, the RNG grammar did not permit <source> unless there was an explicit type= attribute (even though the C code manages just fine by defaulting to type='file'). After making the attribute optional on the 'file' branch, I noticed that the use of diskspec was now redundant with the branch when no <source> was supplied. View this patch with 'git diff -b' for a better picture of the schema change. * docs/schemas/domaincommon.rng (disk): Hoist 'diskspec' out of choice, make type='file' default, and still preserve interleave. * tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-source-pool.xml: * tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-discard.xml: New files. * tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml: * tests/qemuxml2argvdata/qemuxml2argv-disk-drive-discard.xml: Reorder XML. * tests/qemuxml2xmltest.c (mymain): Cover new files. Signed-off-by: Eric Blake <eblake@xxxxxxxxxx> --- docs/schemas/domaincommon.rng | 213 ++++++++++----------- .../qemuxml2argv-disk-drive-discard.xml | 8 +- .../qemuxml2argv-disk-source-pool.xml | 12 +- .../qemuxml2xmlout-disk-drive-discard.xml | 37 ++++ .../qemuxml2xmlout-disk-source-pool.xml | 44 +++++ tests/qemuxml2xmltest.c | 4 +- 6 files changed, 201 insertions(+), 117 deletions(-) create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-discard.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-source-pool.xml diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 8c1724a..7fc0cff 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1196,119 +1196,118 @@ <optional> <ref name="snapshot"/> </optional> - <choice> - <group> - <attribute name="type"> - <value>file</value> - </attribute> - <interleave> + <interleave> + <choice> + <group> <optional> - <element name="source"> - <optional> - <attribute name="file"> - <ref name="absFilePath"/> - </attribute> - </optional> - <optional> - <ref name="startupPolicy"/> - </optional> - <optional> - <ref name='devSeclabel'/> - </optional> - </element> + <attribute name="type"> + <value>file</value> + </attribute> </optional> - <ref name="diskspec"/> - </interleave> - </group> - <group> - <attribute name="type"> - <value>block</value> - </attribute> - <interleave> - <optional> - <element name="source"> - <optional> - <attribute name="dev"> + <interleave> + <optional> + <element name="source"> + <optional> + <attribute name="file"> + <ref name="absFilePath"/> + </attribute> + </optional> + <optional> + <ref name="startupPolicy"/> + </optional> + <optional> + <ref name='devSeclabel'/> + </optional> + </element> + </optional> + </interleave> + </group> + <group> + <attribute name="type"> + <value>block</value> + </attribute> + <interleave> + <optional> + <element name="source"> + <optional> + <attribute name="dev"> + <ref name="absFilePath"/> + </attribute> + </optional> + <optional> + <ref name="startupPolicy"/> + </optional> + <optional> + <ref name='devSeclabel'/> + </optional> + </element> + </optional> + </interleave> + </group> + <group> + <attribute name="type"> + <value>dir</value> + </attribute> + <interleave> + <optional> + <element name="source"> + <attribute name="dir"> <ref name="absFilePath"/> </attribute> - </optional> - <optional> - <ref name="startupPolicy"/> - </optional> - <optional> - <ref name='devSeclabel'/> - </optional> - </element> - </optional> - <ref name="diskspec"/> - </interleave> - </group> - <group> - <attribute name="type"> - <value>dir</value> - </attribute> - <interleave> - <optional> - <element name="source"> - <attribute name="dir"> - <ref name="absFilePath"/> - </attribute> - <optional> - <ref name="startupPolicy"/> - </optional> - <empty/> - </element> - </optional> - <ref name="diskspec"/> - </interleave> - </group> - <group> - <attribute name="type"> - <value>network</value> - </attribute> - <interleave> - <optional> - <element name="source"> - <ref name='diskSourceNetwork'/> - </element> - </optional> - <ref name="diskspec"/> - </interleave> - </group> - <group> - <attribute name="type"> - <value>volume</value> - </attribute> - <interleave> - <optional> - <element name="source"> - <attribute name="pool"> - <ref name="genericName"/> - </attribute> - <attribute name="volume"> - <ref name="volName"/> - </attribute> - <optional> - <attribute name="mode"> - <choice> - <value>host</value> - <value>direct</value> - </choice> + <optional> + <ref name="startupPolicy"/> + </optional> + <empty/> + </element> + </optional> + </interleave> + </group> + <group> + <attribute name="type"> + <value>network</value> + </attribute> + <interleave> + <optional> + <element name="source"> + <ref name='diskSourceNetwork'/> + </element> + </optional> + </interleave> + </group> + <group> + <attribute name="type"> + <value>volume</value> + </attribute> + <interleave> + <optional> + <element name="source"> + <attribute name="pool"> + <ref name="genericName"/> </attribute> - </optional> - <optional> - <ref name="startupPolicy"/> - </optional> - <optional> - <ref name='devSeclabel'/> - </optional> - </element> - </optional> - <ref name="diskspec"/> - </interleave> - </group> + <attribute name="volume"> + <ref name="volName"/> + </attribute> + <optional> + <attribute name="mode"> + <choice> + <value>host</value> + <value>direct</value> + </choice> + </attribute> + </optional> + <optional> + <ref name="startupPolicy"/> + </optional> + <optional> + <ref name='devSeclabel'/> + </optional> + </element> + </optional> + </interleave> + </group> + </choice> <ref name="diskspec"/> - </choice> + </interleave> </element> </define> <define name="diskSourceNetwork"> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-discard.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-discard.xml index f01312f..b15fd63 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-discard.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-discard.xml @@ -16,11 +16,13 @@ <on_crash>restart</on_crash> <devices> <emulator>/usr/bin/qemu</emulator> - <disk type='file' device='disk'> - <driver name='qemu' type='qcow2' discard='unmap'/> + <!-- For this disk, intentionally stress parser resilience to + atypical ordering --> + <disk device='disk'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> <source file='/var/lib/libvirt/images/f14.img'/> <target dev='vda' bus='virtio'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + <driver discard='unmap' name='qemu' type='qcow2'/> </disk> <disk type='file' device='cdrom'> <driver name='qemu' type='raw' discard='ignore'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml index e96f76e..95d5be2 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml @@ -14,15 +14,17 @@ <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu</emulator> - <disk type='volume' device='cdrom'> - <source pool='pool-disk' volume='block+cdrom'> + <!-- For this disk, intentionally stress parser resilience to + atypical ordering --> + <disk device='cdrom' type='volume'> + <address type='drive' controller='0' bus='0' target='0' unit='1'/> + <readonly/> + <target bus='ide' dev='hda'/> + <source volume='block+cdrom' pool='pool-disk'> <seclabel model='selinux' relabel='yes'> <label>system_u:system_r:public_content_t:s0</label> </seclabel> </source> - <target dev='hda' bus='ide'/> - <readonly/> - <address type='drive' controller='0' bus='0' target='0' unit='1'/> </disk> <disk type='volume' device='cdrom'> <driver name='qemu' type='raw'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-discard.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-discard.xml new file mode 100644 index 0000000..f01312f --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-discard.xml @@ -0,0 +1,37 @@ +<domain type='qemu'> + <name>test</name> + <uuid>92d7a226-cfae-425b-a6d3-00bbf9ec5c9e</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-0.13'>hvm</type> + <boot dev='cdrom'/> + <boot dev='hd'/> + <bootmenu enable='yes'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2' discard='unmap'/> + <source file='/var/lib/libvirt/images/f14.img'/> + <target dev='vda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </disk> + <disk type='file' device='cdrom'> + <driver name='qemu' type='raw' discard='ignore'/> + <source file='/var/lib/libvirt/Fedora-14-x86_64-Live-KDE.iso'/> + <target dev='hdc' bus='ide'/> + <readonly/> + <address type='drive' controller='0' bus='1' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-source-pool.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-source-pool.xml new file mode 100644 index 0000000..e96f76e --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-source-pool.xml @@ -0,0 +1,44 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='volume' device='cdrom'> + <source pool='pool-disk' volume='block+cdrom'> + <seclabel model='selinux' relabel='yes'> + <label>system_u:system_r:public_content_t:s0</label> + </seclabel> + </source> + <target dev='hda' bus='ide'/> + <readonly/> + <address type='drive' controller='0' bus='0' target='0' unit='1'/> + </disk> + <disk type='volume' device='cdrom'> + <driver name='qemu' type='raw'/> + <target dev='hdc' bus='ide'/> + <readonly/> + <address type='drive' controller='0' bus='1' target='0' unit='0'/> + </disk> + <disk type='file' device='disk'> + <source file='/tmp/idedisk.img'/> + <target dev='hdc' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='2'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='ide' index='1'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index c8a1c10..788adbe 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -282,10 +282,10 @@ mymain(void) DO_TEST("disk-scsi-lun-passthrough-sgio"); DO_TEST("disk-scsi-disk-vpd"); - DO_TEST("disk-source-pool"); + DO_TEST_DIFFERENT("disk-source-pool"); DO_TEST("disk-source-pool-mode"); - DO_TEST("disk-drive-discard"); + DO_TEST_DIFFERENT("disk-drive-discard"); DO_TEST("virtio-rng-random"); DO_TEST("virtio-rng-egd"); -- 1.9.0 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list