From bbd392ff86249434ca63bb5977d2b1bd2d72a538 Mon Sep 17 00:00:00 2001
From: Peter Krempa <pkrempa@redhat.com>
Date: Tue, 4 Feb 2014 19:02:04 +0100
Subject: [PATCH] schema: Fix guest timer specification schema according to the
 docs

According to the documentation describing various tunables for domain
timers not all the fields are supported by all the driver types. Express
these in the RNG:

- rtc, platform: Only these support the "track" attribute.
- tsc: only one to support "frequency" and "mode" attributes
- hpet, pit: tickpolicy/catchup attribute/element
- kvmclock: no extra attributes are supported

Additionally the attributes of the <catchup> element for
tickpolicy='catchup' are optional according to the parsing code. Express
this in the XML and fix a spurious space added while formatting the
<catchup> element and add tests for it.
---
 docs/schemas/domaincommon.rng                 | 157 +++++++++++-------
 src/conf/domain_conf.c                        |   2 +-
 .../qemuxml2argv-clock-catchup.args           |   5 +
 .../qemuxml2argv-clock-catchup.xml            |  37 +++++
 tests/qemuxml2argvtest.c                      |   1 +
 tests/qemuxml2xmltest.c                       |   1 +
 6 files changed, 142 insertions(+), 61 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-clock-catchup.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-clock-catchup.xml

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 7f55f24fc0..63e02e4b14 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -857,54 +857,68 @@
   </define>
   <define name="timer">
     <element name="timer">
-      <attribute name="name">
-        <choice>
-          <value>platform</value>
-          <value>hpet</value>
-          <value>kvmclock</value>
-          <value>pit</value>
-          <value>rtc</value>
-          <value>tsc</value>
-        </choice>
-      </attribute>
-      <optional>
-        <attribute name="track">
-          <choice>
-            <value>boot</value>
-            <value>guest</value>
-            <value>wall</value>
-          </choice>
-        </attribute>
-      </optional>
-      <optional>
-        <attribute name="tickpolicy">
-          <choice>
-            <value>delay</value>
-            <value>catchup</value>
-            <value>merge</value>
-            <value>discard</value>
-          </choice>
-        </attribute>
-      </optional>
-      <optional>
-        <ref name="catchup"/>
-      </optional>
-      <optional>
-        <attribute name="frequency">
-          <ref name="unsignedInt"/>
-        </attribute>
-      </optional>
-      <optional>
-        <attribute name="mode">
-          <choice>
-            <value>auto</value>
-            <value>native</value>
-            <value>emulate</value>
-            <value>paravirt</value>
-            <value>smpsafe</value>
-          </choice>
-        </attribute>
-      </optional>
+      <choice>
+        <group>
+          <attribute name="name">
+            <choice>
+              <value>platform</value>
+              <value>rtc</value>
+            </choice>
+          </attribute>
+          <optional>
+            <attribute name="track">
+              <choice>
+                <value>boot</value>
+                <value>guest</value>
+                <value>wall</value>
+              </choice>
+            </attribute>
+          </optional>
+          <optional>
+            <ref name="tickpolicy"/>
+          </optional>
+        </group>
+        <group>
+          <attribute name="name">
+            <value>tsc</value>
+          </attribute>
+          <optional>
+            <ref name="tickpolicy"/>
+          </optional>
+          <optional>
+            <attribute name="frequency">
+              <ref name="unsignedInt"/>
+            </attribute>
+          </optional>
+          <optional>
+            <attribute name="mode">
+              <choice>
+                <value>auto</value>
+                <value>native</value>
+                <value>emulate</value>
+                <value>paravirt</value>
+                <value>smpsafe</value>
+              </choice>
+            </attribute>
+          </optional>
+        </group>
+        <group>
+          <attribute name="name">
+            <choice>
+              <value>hpet</value>
+              <value>pit</value>
+            </choice>
+          </attribute>
+          <optional>
+            <ref name="tickpolicy"/>
+          </optional>
+        </group>
+        <group>
+          <attribute name="name">
+            <value>kvmclock</value>
+          </attribute>
+        </group>
+      </choice>
       <optional>
         <attribute name="present">
           <choice>
@@ -916,20 +930,43 @@
       <empty/>
     </element>
   </define>
-  <define name="catchup">
-    <element name="catchup">
-      <optional>
-        <attribute name="threshold">
-          <ref name="unsignedInt"/>
-        </attribute>
-        <attribute name="slew">
-          <ref name="unsignedInt"/>
+
+  <define name="tickpolicy">
+    <choice>
+      <group>
+        <attribute name="tickpolicy">
+          <choice>
+            <value>delay</value>
+            <value>merge</value>
+            <value>discard</value>
+          </choice>
         </attribute>
-        <attribute name="limit">
-          <ref name="unsignedInt"/>
+      </group>
+      <group>
+        <attribute name="tickpolicy">
+          <value>catchup</value>
         </attribute>
-      </optional>
-    </element>
+        <optional>
+          <element name="catchup">
+            <optional>
+              <attribute name="threshold">
+                <ref name="unsignedInt"/>
+              </attribute>
+            </optional>
+            <optional>
+              <attribute name="slew">
+                <ref name="unsignedInt"/>
+              </attribute>
+            </optional>
+            <optional>
+              <attribute name="limit">
+                <ref name="unsignedInt"/>
+              </attribute>
+            </optional>
+          </element>
+        </optional>
+      </group>
+    </choice>
   </define>
 <!--
       A bootloader may be used to extract the OS information instead of
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 512fe51520..94af3e6697 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16225,7 +16225,7 @@ virDomainTimerDefFormat(virBufferPtr buf,
         virBufferAddLit(buf, "/>\n");
     } else {
         virBufferAddLit(buf, ">\n");
-        virBufferAddLit(buf, "      <catchup ");
+        virBufferAddLit(buf, "      <catchup");
         if (def->catchup.threshold > 0) {
             virBufferAsprintf(buf, " threshold='%lu'", def->catchup.threshold);
         }
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-clock-catchup.args b/tests/qemuxml2argvdata/qemuxml2argv-clock-catchup.args
new file mode 100644
index 0000000000..c238a1332b
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-clock-catchup.args
@@ -0,0 +1,5 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic \
+-monitor unix:/tmp/test-monitor,server,nowait -rtc base=utc,driftfix=slew \
+-no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 \
+-net none -serial none -parallel none
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-clock-catchup.xml b/tests/qemuxml2argvdata/qemuxml2argv-clock-catchup.xml
new file mode 100644
index 0000000000..7b3cb88f22
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-clock-catchup.xml
@@ -0,0 +1,37 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219100</memory>
+  <currentMemory unit='KiB'>219100</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'>
+    <timer name='rtc' tickpolicy='catchup'>
+      <catchup slew='4636'/>
+    </timer>
+    <timer name='pit' tickpolicy='catchup'>
+      <catchup threshold='123'/>
+    </timer>
+    <timer name='hpet' tickpolicy='catchup'>
+      <catchup limit='2342'/>
+    </timer>
+  </clock>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu</emulator>
+    <disk type='block' device='disk'>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' 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/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index dae08d52f5..8f26618cc9 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -633,6 +633,7 @@ mymain(void)
     DO_TEST("clock-france", QEMU_CAPS_RTC);
     DO_TEST("clock-hpet-off", QEMU_CAPS_RTC, QEMU_CAPS_NO_HPET,
             QEMU_CAPS_NO_KVM_PIT);
+    DO_TEST("clock-catchup", QEMU_CAPS_RTC, QEMU_CAPS_NO_KVM_PIT);
     DO_TEST("cpu-kvmclock", QEMU_CAPS_ENABLE_KVM);
     DO_TEST("cpu-host-kvmclock", QEMU_CAPS_ENABLE_KVM, QEMU_CAPS_CPU_HOST);
     DO_TEST("kvmclock", QEMU_CAPS_KVM);
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 41d19047d0..0270eee5c7 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -158,6 +158,7 @@ mymain(void)
     DO_TEST("clock-localtime");
     DO_TEST("cpu-kvmclock");
     DO_TEST("cpu-host-kvmclock");
+    DO_TEST("clock-catchup");
     DO_TEST("kvmclock");
 
     DO_TEST("cpu-eoi-disabled");
-- 
GitLab