[libvirt] [PATCH 3/3] conf: Fix perf event parser

Peter Krempa pkrempa at redhat.com
Wed Jun 15 15:15:16 UTC 2016


The parser was totaly broken. Fix it by rewriting it. Add tests so that
it doesn't happen.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1346723
---
 src/conf/domain_conf.c                      | 47 ++++++++++++-----------------
 tests/genericxml2xmlindata/generic-perf.xml | 22 ++++++++++++++
 tests/genericxml2xmltest.c                  |  2 ++
 3 files changed, 43 insertions(+), 28 deletions(-)
 create mode 100644 tests/genericxml2xmlindata/generic-perf.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9504e5f..215538a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13056,57 +13056,48 @@ virDomainPMStateParseXML(xmlXPathContextPtr ctxt,

 static int
 virDomainPerfEventDefParseXML(virDomainPerfDefPtr perf,
-                              xmlNodePtr node,
-                              xmlXPathContextPtr ctxt)
+                              xmlNodePtr node)
 {
     char *name = NULL;
     char *enabled = NULL;
-    int enabled_type;
-    int name_type;
+    int event;
     int ret = -1;

-    xmlNodePtr oldnode = ctxt->node;
-
-    ctxt->node = node;
-    if (!(name = virXPathString("string(./@name)", ctxt))) {
-        virReportError(VIR_ERR_CONF_SYNTAX, "%s",
-                       _("missing name for event"));
+    if (!(name = virXMLPropString(node, "name"))) {
+        virReportError(VIR_ERR_XML_ERROR, "%s", _("missing perf event name"));
         goto cleanup;
     }

-    if ((name_type = virPerfEventTypeFromString(name)) < 0) {
-        virReportError(VIR_ERR_CONF_SYNTAX,
-                       _("%s is not a supported event name"), name);
+    if ((event = virPerfEventTypeFromString(name)) < 0) {
+        virReportError(VIR_ERR_XML_ERROR,
+                       _("'unsupported perf event '%s'"), name);
         goto cleanup;
     }

-    if (!(enabled = virXPathString("string(./@enabled)", ctxt))) {
-        virReportError(VIR_ERR_CONF_SYNTAX,
-                       _("missing state for cipher named %s"), name);
+    if (perf->events[event] != VIR_TRISTATE_BOOL_ABSENT) {
+        virReportError(VIR_ERR_XML_ERROR,
+                       _("perf event '%s' was already specified"), name);
         goto cleanup;
     }

-    if ((enabled_type = virTristateBoolTypeFromString(enabled)) < 0) {
-        virReportError(VIR_ERR_CONF_SYNTAX,
-                       _("%s is not a supported enabled state"), enabled);
+    if (!(enabled = virXMLPropString(node, "enabled"))) {
+        virReportError(VIR_ERR_XML_ERROR,
+                       _("missing state of perf event '%s'"), name);
         goto cleanup;
     }

-    if (perf->events[VIR_PERF_EVENT_CMT] != VIR_TRISTATE_BOOL_ABSENT) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("A domain definition can have no more than "
-                         "one event node with name %s"),
-                       virTristateBoolTypeToString(name_type));
-
+    if ((perf->events[event] = virTristateBoolTypeFromString(enabled)) < 0) {
+        virReportError(VIR_ERR_XML_ERROR,
+                       _("invalid state '%s' of perf event '%s'"),
+                       enabled, name);
         goto cleanup;
     }
-    perf->events[VIR_PERF_EVENT_CMT] = enabled_type;

     ret = 0;
+
  cleanup:
     VIR_FREE(name);
     VIR_FREE(enabled);
-    ctxt->node = oldnode;
     return ret;
 }

@@ -13126,7 +13117,7 @@ virDomainPerfDefParseXML(virDomainDefPtr def,
         goto cleanup;

     for (i = 0; i < n; i++) {
-        if (virDomainPerfEventDefParseXML(def->perf, nodes[i], ctxt) < 0)
+        if (virDomainPerfEventDefParseXML(def->perf, nodes[i]) < 0)
             goto cleanup;
     }

diff --git a/tests/genericxml2xmlindata/generic-perf.xml b/tests/genericxml2xmlindata/generic-perf.xml
new file mode 100644
index 0000000..394d2a6
--- /dev/null
+++ b/tests/genericxml2xmlindata/generic-perf.xml
@@ -0,0 +1,22 @@
+<domain type='qemu'>
+  <name>foo</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>
+  <perf>
+    <event name='cmt' enabled='yes'/>
+    <event name='mbmt' enabled='no'/>
+    <event name='mbml' enabled='yes'/>
+  </perf>
+  <devices>
+  </devices>
+</domain>
diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c
index d492fb4..1a7a668 100644
--- a/tests/genericxml2xmltest.c
+++ b/tests/genericxml2xmltest.c
@@ -95,6 +95,8 @@ mymain(void)
     DO_TEST_FULL("name-slash-parse", 0, false,
         TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE);

+    DO_TEST("perf");
+
     virObjectUnref(caps);
     virObjectUnref(xmlopt);

-- 
2.8.3




More information about the libvir-list mailing list