Change in vdsm[master]: WIP virt: use (c)ElementTree to process XML

fromani at redhat.com fromani at redhat.com
Wed Apr 16 15:17:55 UTC 2014


Francesco Romani has uploaded a new change for review.

Change subject: WIP virt: use (c)ElementTree to process XML
......................................................................

WIP virt: use (c)ElementTree to process XML

this let the virt internal machinery to use (c)ElementTree
instead of minidom. The main driver is performance,
however a nice side effects is the API is a little bit nicer.

This patch is still very much Work In Progress and still
needs quite a lot of polishing and, most likely, to be
split up.

Change-Id: I7874026acf52b869b8329f433d5833530e0d02e0
Signed-off-by: Francesco Romani <fromani at redhat.com>
---
M lib/vdsm/compat.py
M tests/vmTests.py
M vdsm/virt/vm.py
M vdsm/virt/xmldom.py
4 files changed, 226 insertions(+), 282 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/56/26856/1

diff --git a/lib/vdsm/compat.py b/lib/vdsm/compat.py
index 26c558e..a59a034 100644
--- a/lib/vdsm/compat.py
+++ b/lib/vdsm/compat.py
@@ -24,3 +24,10 @@
 except ImportError:  # py3
     import pickle
     pickle  # yep, this is needed twice.
+
+try:
+    import xml.etree.cElementTree as ET
+    ET  # make pyflakes happy
+except ImportError:  # py3
+    import xml.etree.ElementTree as ET
+    ET  # once again, needed twice.
diff --git a/tests/vmTests.py b/tests/vmTests.py
index d655286..20b1007 100644
--- a/tests/vmTests.py
+++ b/tests/vmTests.py
@@ -61,36 +61,43 @@
 
     def assertXML(self, element, expectedXML, path=None):
         if path is None:
-            converted = element.toprettyxml()
+            converted = ET.tostring(element, encoding='utf-8', method='xml')
         else:
-            elem = ET.fromstring(element.toprettyxml())
-            converted = re.sub(' />', '/>',
-                               ET.tostring(elem.find("./%s" % path)))
-        self.assertEqual(re.sub('\n\s*', ' ', converted).strip(' '),
-                         re.sub('\n\s*', ' ', expectedXML).strip(' '))
+            converted = ET.tostring(element.find("./%s" % path))
+        self._assertEqualXML(converted, expectedXML)
 
-    def assertXMLNone(self, element, path):
-        elem = ET.fromstring(element.toprettyxml())
-        converted = elem.find("./%s" % path)
-        self.assertEqual(converted, None)
+    def _minimizeXML(self, xml):
+        xml = xml.strip()
+        xml = re.sub('\n\s*', ' ', xml)
+        xml = re.sub(' />', '/>', xml)
+        xml = re.sub('>\s*', '>', xml)
+        return xml
+
+    def _assertEqualXML(self, output, expected):
+        out = self._minimizeXML(output)
+        exp = self._minimizeXML(expected)
+        if out != exp:
+            for i, pair in enumerate(zip(out, exp)):
+                if pair[0] != pair[-1]:
+                    caret = '%s^' % ('-' * i)
+                    break
+            print out
+            print exp
+            print caret
+            self.assertEqual(out, exp)
 
     def assertBuildCmdLine(self, confToDom):
-        oldVdsmRun = constants.P_VDSM_RUN
-        constants.P_VDSM_RUN = tempfile.mkdtemp()
-        try:
-            for conf, expectedXML in confToDom:
+        with namedTemporaryDir() as tmpDir:
+            with MonkeyPatchScope([(constants, 'P_VDSM_RUN', tmpDir + '/')]):
+                for conf, expectedXML in confToDom:
 
-                expectedXML = expectedXML % conf
+                    expectedXML = expectedXML % conf
 
-                testVm = vm.Vm(self, conf)
+                    testVm = vm.Vm(self, conf)
 
-                output = testVm._buildCmdLine()
+                    output = testVm._buildCmdLine()
 
-                self.assertEqual(re.sub('\n\s*', ' ', output.strip(' ')),
-                                 re.sub('\n\s*', ' ', expectedXML.strip(' ')))
-        finally:
-            shutil.rmtree(constants.P_VDSM_RUN)
-            constants.P_VDSM_RUN = oldVdsmRun
+                    self._assertEqualXML(output, expectedXML)
 
     def testDomXML(self):
         expectedXML = """
@@ -519,10 +526,11 @@
                'custom': {'queues': '7'}}
         self.conf['custom'] = {'vhost': 'ovirtmgmt:true', 'sndbuf': '0'}
         iface = vm.NetworkInterfaceDevice(self.conf, self.log, **dev)
-        originalBandwidth = iface.getXML().getElementsByTagName('bandwidth')[0]
+        originalBandwidth = iface.getXML().find('bandwidth')
         self.assertXML(originalBandwidth, originalBwidthXML)
-        self.assertXML(iface.paramsToBandwidthXML(NEW_OUT, originalBandwidth),
-                       updatedBwidthXML)
+        updatedBandwidth = iface.paramsToBandwidthXML(
+            NEW_OUT, originalBandwidth)
+        self.assertXML(updatedBandwidth, updatedBwidthXML)
 
     def testControllerXML(self):
         devConfs = [
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index a68f101..75924b3 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -484,7 +484,7 @@
         ctrl = self.createXmlElem('controller', self.device,
                                   ['index', 'model', 'master', 'address'])
         if self.device == 'virtio-serial':
-            ctrl.setAttrs(index='0', ports='16')
+            xmldom.setattrs(ctrl, index='0', ports='16')
 
         return ctrl
 
@@ -502,7 +502,7 @@
         if 'ram' in self.specParams:
             sourceAttrs['ram'] = self.specParams['ram']
 
-        video.appendChildWithArgs('model', type=self.device, **sourceAttrs)
+        xmldom.appendElement(video, 'model', type=self.device, **sourceAttrs)
         return video
 
 
@@ -514,7 +514,7 @@
         Create domxml for sound device
         """
         sound = self.createXmlElem('sound', None, ['address'])
-        sound.setAttrs(model=self.device)
+        sound.set('model', self.device)
         return sound
 
 
@@ -588,46 +588,48 @@
         </interface>
         """
         iface = self.createXmlElem('interface', self.device, ['address'])
-        iface.appendChildWithArgs('mac', address=self.macAddr)
-        iface.appendChildWithArgs('model', type=self.nicModel)
-        iface.appendChildWithArgs('source', bridge=self.network)
+        xmldom.appendElement(iface, 'mac', address=self.macAddr)
+        xmldom.appendElement(iface, 'model', type=self.nicModel)
+        xmldom.appendElement(iface, 'source', bridge=self.network)
         if hasattr(self, 'filter'):
-            iface.appendChildWithArgs('filterref', filter=self.filter)
+            xmldom.appendElement(iface, 'filterref', filter=self.filter)
 
         if hasattr(self, 'linkActive'):
-            iface.appendChildWithArgs('link', state='up'
-                                      if utils.tobool(self.linkActive)
-                                      else 'down')
+            xmldom.appendElement(iface, 'link',
+                                state='up'
+                                if utils.tobool(self.linkActive)
+                                else 'down')
 
         if hasattr(self, 'bootOrder'):
-            iface.appendChildWithArgs('boot', order=self.bootOrder)
+            xmldom.appendElement(iface, 'boot', order=self.bootOrder)
 
         if self.driver:
-            iface.appendChildWithArgs('driver', **self.driver)
+            xmldom.appendElement(iface, 'driver', self.driver)
 
         if self.sndbufParam:
-            tune = iface.appendChildWithArgs('tune')
-            tune.appendChildWithArgs('sndbuf', text=self.sndbufParam)
+            tune = xmldom.appendElement(iface, 'tune')
+            xmldom.appendElement(tune, 'sndbuf').text=self.sndbufParam
 
         if hasattr(self, 'specParams'):
             if 'inbound' in self.specParams or 'outbound' in self.specParams:
-                iface.appendChild(self.paramsToBandwidthXML(self.specParams))
+                iface.append(self.paramsToBandwidthXML(self.specParams))
         return iface
 
     def paramsToBandwidthXML(self, specParams, oldBandwidth=None):
         """Returns a valid libvirt xml dom element object."""
         bandwidth = self.createXmlElem('bandwidth', None)
-        old = {} if oldBandwidth is None else dict(
-            (elem.nodeName, elem) for elem in oldBandwidth.childNodes)
+        if oldBandwidth is None:
+            oldBandwidth = xmldom.Element('bandwidth')
         for key in ('inbound', 'outbound'):
             elem = specParams.get(key)
             if elem is None:  # Use the old setting if present
-                if key in old:
-                    bandwidth.appendChild(old[key])
+                value = oldBandwidth.find(key)
+                if value is not None:
+                    bandwidth.append(value)
             elif elem:
                 # Convert the values to string for adding them to the XML def
                 attrs = dict((key, str(value)) for key, value in elem.items())
-                bandwidth.appendChildWithArgs(key, **attrs)
+                xmldom.appendElement(bandwidth, key, attrs)
         return bandwidth
 
 
@@ -836,12 +838,13 @@
         # when libvirt will support shared leases this will loop over all the
         # volumes
         for volInfo in self.volumeChain[-1:]:
-            lease = xmldom.XMLElement('lease')
-            lease.appendChildWithArgs('key', text=volInfo['volumeID'])
-            lease.appendChildWithArgs('lockspace',
-                                      text=volInfo['domainID'])
-            lease.appendChildWithArgs('target', path=volInfo['leasePath'],
-                                      offset=str(volInfo['leaseOffset']))
+            lease = xmldom.Element('lease')
+            xmldom.appendElement(lease, 'key').text=volInfo['volumeID']
+            xmldom.appendElement(lease, 'lockspace').text=\
+                                      text=volInfo['domainID']
+            xmldom.appendElement(lease, 'target',
+                path=volInfo['leasePath'],
+                offset=str(volInfo['leaseOffset']))
             yield lease
 
     def getXML(self):
@@ -857,42 +860,43 @@
         """
         self.device = getattr(self, 'device', 'disk')
 
-        source = xmldom.XMLElement('source')
+        source = xmldom.Element('source')
         if self.blockDev:
             deviceType = 'block'
-            source.setAttrs(dev=self.path)
+            source.set('dev', self.path)
         elif self.networkDev:
             deviceType = 'network'
-            source.setAttrs(protocol=self.volumeInfo['protocol'],
+            xmldom.setattrs(source,
+                            protocol=self.volumeInfo['protocol'],
                             name=self.volumeInfo['path'])
             hostAttrs = {'name': self.volumeInfo['volfileServer'],
                          'port': self.volumeInfo['volPort'],
                          'transport': self.volumeInfo['volTransport']}
-            source.appendChildWithArgs('host', **hostAttrs)
+            xmldom.appendElement(source, 'host', hostAttrs)
         else:
             deviceType = 'file'
             sourceAttrs = {'file': self.path}
             if self.device == 'cdrom' or self.device == 'floppy':
                 sourceAttrs['startupPolicy'] = 'optional'
-            source.setAttrs(**sourceAttrs)
+            xmldom.setattrs(source, sourceAttrs)
         diskelem = self.createXmlElem('disk', deviceType,
                                       ['device', 'address', 'sgio'])
-        diskelem.setAttrs(snapshot='no')
-        diskelem.appendChild(source)
+        diskelem.set('snapshot', 'no')
+        diskelem.append(source)
 
         targetAttrs = {'dev': self.name}
         if self.iface:
             targetAttrs['bus'] = self.iface
-        diskelem.appendChildWithArgs('target', **targetAttrs)
+        xmldom.appendElement(diskelem, 'target', targetAttrs)
 
         if self.extSharedState == DRIVE_SHARED_TYPE.SHARED:
-            diskelem.appendChildWithArgs('shareable')
+            xmldom.appendElement(diskelem, 'shareable')
         if hasattr(self, 'readonly') and utils.tobool(self.readonly):
-            diskelem.appendChildWithArgs('readonly')
+            xmldom.appendElement(diskelem, 'readonly')
         if hasattr(self, 'serial'):
-            diskelem.appendChildWithArgs('serial', text=self.serial)
+            xmldom.appendElement(diskelem, 'serial').text=self.serial
         if hasattr(self, 'bootOrder'):
-            diskelem.appendChildWithArgs('boot', order=self.bootOrder)
+            xmldom.appendElement(diskelem, 'boot', order=self.bootOrder)
 
         if self.device != 'lun' and hasattr(self, 'sgio'):
             raise ValueError("sgio attribute can be set only for LUN devices")
@@ -918,19 +922,18 @@
                 driverAttrs['error_policy'] = 'enospace'
             else:
                 driverAttrs['error_policy'] = 'stop'
-            diskelem.appendChildWithArgs('driver', **driverAttrs)
+            xmldom.appendElement(diskelem, 'driver', driverAttrs)
         elif self.device == 'floppy':
             if (self.path and
                 not utils.getUserPermissions(constants.QEMU_PROCESS_USER,
                                              self.path)['write']):
-                diskelem.appendChildWithArgs('readonly')
+                xmldom.appendElement(diskelem, 'readonly')
 
         if hasattr(self, 'specParams') and 'ioTune' in self.specParams:
             self._validateIoTuneParams()
-            iotune = xmldom.XMLElement('iotune')
+            iotune = xmldom.appendElement(diskelem, 'iotune')
             for key, value in self.specParams['ioTune'].iteritems():
-                iotune.appendChildWithArgs(key, text=str(value))
-            diskelem.appendChild(iotune)
+                xmldom.appendElement(iotune, key).text=str(value)
 
         return diskelem
 
@@ -947,9 +950,8 @@
            function='0x0'/>
         </memballoon>
         """
-        m = self.createXmlElem(self.device, None, ['address'])
-        m.setAttrs(model=self.specParams['model'])
-        return m
+        balloon = self.createXmlElem(self.device, None, ['address'])
+        return xmldom.setattrs(balloon, model=self.specParams['model'])
 
 
 class WatchdogDevice(VmDevice):
@@ -970,10 +972,10 @@
            function='0x0'/>
         </watchdog>
         """
-        m = self.createXmlElem(self.type, None, ['address'])
-        m.setAttrs(model=self.specParams.get('model', 'i6300esb'),
-                   action=self.specParams.get('action', 'none'))
-        return m
+        dev = self.createXmlElem(self.type, None, ['address'])
+        return xmldom.setattrs(dev,
+            model=self.specParams.get('model', 'i6300esb'),
+            action=self.specParams.get('action', 'none'))
 
 
 class SmartCardDevice(VmDevice):
@@ -991,8 +993,7 @@
         sourceAttrs = {'mode': self.specParams['mode']}
         if sourceAttrs['mode'] != 'host':
             sourceAttrs['type'] = self.specParams['type']
-        card.setAttrs(**sourceAttrs)
-        return card
+        return xmldom.setattrs(card, sourceAttrs)
 
 
 class TpmDevice(VmDevice):
@@ -1009,11 +1010,11 @@
         </tpm>
         """
         tpm = self.createXmlElem(self.device, None)
-        tpm.setAttrs(model=self.specParams['model'])
-        backend = tpm.appendChildWithArgs('backend',
+        tpm.set('model', self.specParams['model'])
+        backend = xmldom.appendElement(tpm, 'backend',
                                           type=self.specParams['mode'])
-        backend.appendChildWithArgs('device',
-                                    path=self.specParams['path'])
+        xmldom.appendElement(backend, 'device',
+            path=self.specParams['path'])
         return tpm
 
 
@@ -1047,12 +1048,11 @@
             if 'period' in self.specParams:
                 rateAttrs['period'] = self.specParams['period']
 
-            rng.appendChildWithArgs('rate', None, **rateAttrs)
+            xmldom.appendElement(rng, 'rate', rateAttrs)
 
         # <backend... /> element
-        rng.appendChildWithArgs('backend',
-                                caps.RNG_SOURCES[self.specParams['source']],
-                                model='random')
+        xmldom.appendElement(rng, 'backend', model='random').text=\
+                                caps.RNG_SOURCES[self.specParams['source']]
 
         return rng
 
@@ -1065,12 +1065,12 @@
         Create domxml for a console device.
 
         <console type='pty'>
-          <target type='virtio' port='0'/>
+          <target type='irtio' port='0'/>
         </console>
         """
-        m = self.createXmlElem('console', 'pty')
-        m.appendChildWithArgs('target', type='virtio', port='0')
-        return m
+        console = self.createXmlElem('console', 'pty')
+        xmldom.appendElement(console, 'target', type='virtio', port='0')
+        return console
 
 
 class MigrationError(Exception):
@@ -2134,14 +2134,15 @@
 
         for devType in self._devices:
             for dev in self._devices[devType]:
-                deviceXML = dev.getXML().toxml(encoding='utf-8')
+                xmlDev = dev.getXML()
+                deviceXML = dev.toXML()
 
                 if getattr(dev, "custom", {}):
                     deviceXML = hooks.before_device_create(
                         deviceXML, self.conf, dev.custom)
 
                 dev._deviceXML = deviceXML
-                domxml._devices.appendChild(xmldom.parse(deviceXML).firstChild)
+                domxml._devices.append(xmlDev)
 
     def _buildCmdLine(self):
         domxml = xmldom._DomXML(self.conf, self.log, self.arch)
@@ -2550,24 +2551,22 @@
     def setLinkAndNetwork(self, dev, conf, linkValue, networkValue, custom,
                           specParams=None):
         vnicXML = dev.getXML()
-        source = vnicXML.getElementsByTagName('source')[0]
-        source.setAttribute('bridge', networkValue)
-        try:
-            link = vnicXML.getElementsByTagName('link')[0]
-        except IndexError:
-            link = xmldom.XMLElement('link')
-            vnicXML.appendChildWithArgs(link)
-        link.setAttribute('state', linkValue)
+        source = vnicXML.find('source')
+        source.set('bridge', networkValue)
+        link = vnicXML.find('link')
+        if link is None:
+            link = xmldom.appendElement(vnicXML, 'link')
+        link.set('state', linkValue)
         if (specParams and
                 ('inbound' in specParams or 'outbound' in specParams)):
-            oldBandwidths = vnicXML.getElementsByTagName('bandwidth')
-            oldBandwidth = oldBandwidths[0] if len(oldBandwidths) else None
+            oldBandwidth = vnicXML.find('bandwidth')
             newBandwidth = dev.paramsToBandwidthXML(specParams, oldBandwidth)
             if oldBandwidth is None:
-                vnicXML.appendChild(newBandwidth)
+                vnicXML.append(newBandwidth)
             else:
-                vnicXML.replaceChild(newBandwidth, oldBandwidth)
-        vnicStrXML = vnicXML.toprettyxml(encoding='utf-8')
+                vnicXML.remove(oldBandwidth)
+                vnicXML.append(newBandwidth)
+        vnicStrXML = vnicXML.toXML()
         try:
             try:
                 vnicStrXML = hooks.before_update_device(vnicStrXML, self.conf,
@@ -2586,7 +2585,7 @@
             # Rollback link and network.
             self.log.debug('Rolling back link and net for: %s', dev.alias,
                            exc_info=True)
-            self._dom.updateDeviceFlags(vnicXML.toxml(encoding='utf-8'),
+            self._dom.updateDeviceFlags(vnicXML.toXML(),
                                         libvirt.VIR_DOMAIN_AFFECT_LIVE)
             raise
         else:
diff --git a/vdsm/virt/xmldom.py b/vdsm/virt/xmldom.py
index ea1387c..e15c2ca 100644
--- a/vdsm/virt/xmldom.py
+++ b/vdsm/virt/xmldom.py
@@ -25,6 +25,7 @@
 import xml.dom.minidom
 
 # vdsm imports
+from vdsm.compat import ET
 from vdsm import constants
 from vdsm import netinfo
 from vdsm import utils
@@ -32,6 +33,10 @@
 # local imports
 # In future those should be imported via ..
 import caps
+
+
+Element = ET.Element  # FIXME
+appendElement = ET.SubElement
 
 
 def is_node(xmlNode):
@@ -71,34 +76,6 @@
         return self._hash or '0'
 
 
-class XMLElement(object):
-
-    def __init__(self, tagName, text=None, **attrs):
-        self._elem = xml.dom.minidom.Document().createElement(tagName)
-        self.setAttrs(**attrs)
-        if text is not None:
-            self.appendTextNode(text)
-
-    def __getattr__(self, name):
-        return getattr(self._elem, name)
-
-    def setAttrs(self, **attrs):
-        for attrName, attrValue in attrs.iteritems():
-            self._elem.setAttribute(attrName, attrValue)
-
-    def appendTextNode(self, text):
-        textNode = xml.dom.minidom.Document().createTextNode(text)
-        self._elem.appendChild(textNode)
-
-    def appendChild(self, element):
-        self._elem.appendChild(element)
-
-    def appendChildWithArgs(self, childName, text=None, **attrs):
-        child = XMLElement(childName, text, **attrs)
-        self._elem.appendChild(child)
-        return child
-
-
 class _DomXML:
     def __init__(self, conf, log, arch):
         """
@@ -123,8 +100,6 @@
 
         self.arch = arch
 
-        self.doc = xml.dom.minidom.Document()
-
         if utils.tobool(self.conf.get('kvmEnable', 'true')):
             domainType = 'kvm'
         else:
@@ -140,29 +115,22 @@
             domainAttrs['xmlns:qemu'] = \
                 'http://libvirt.org/schemas/domain/qemu/1.0'
 
-        self.dom = XMLElement('domain', **domainAttrs)
-        self.doc.appendChild(self.dom)
-
-        self.dom.appendChildWithArgs('name', text=self.conf['vmName'])
-        self.dom.appendChildWithArgs('uuid', text=self.conf['vmId'])
+        self.dom = ET.Element('domain', domainAttrs)
+        appendElement(self.dom, 'name').text=self.conf['vmName']
+        appendElement(self.dom, 'uuid').text=self.conf['vmId']
         memSizeKB = str(int(self.conf.get('memSize', '256')) * 1024)
-        self.dom.appendChildWithArgs('memory', text=memSizeKB)
-        self.dom.appendChildWithArgs('currentMemory', text=memSizeKB)
-        vcpu = self.dom.appendChildWithArgs('vcpu', text=self._getMaxVCpus())
-        vcpu.setAttrs(**{'current': self._getSmp()})
+        appendElement(self.dom, 'memory').text=memSizeKB
+        appendElement(self.dom, 'currentMemory').text=memSizeKB
+        appendElement(self.dom, 'vcpu', current=self._getSmp()).text=self._getMaxVCpus()
 
         memSizeGuaranteedKB = str(1024 * int(
             self.conf.get('memGuaranteedSize', '0')
         ))
 
-        memtune = XMLElement('memtune')
-        self.dom.appendChild(memtune)
+        memtune = appendElement(self.dom, 'memtune')
+        appendElement(memtune, 'min_guarantee').text=memSizeGuaranteedKB
 
-        memtune.appendChildWithArgs('min_guarantee',
-                                    text=memSizeGuaranteedKB)
-
-        self._devices = XMLElement('devices')
-        self.dom.appendChild(self._devices)
+        self._devices = appendElement(self.dom, 'devices')
 
     def appendClock(self):
         """
@@ -173,15 +141,13 @@
         </clock>
         """
 
-        m = XMLElement('clock', offset='variable',
-                       adjustment=str(self.conf.get('timeOffset', 0)))
-        m.appendChildWithArgs('timer', name='rtc', tickpolicy='catchup')
-        m.appendChildWithArgs('timer', name='pit', tickpolicy='delay')
-
+        clock = appendElement(self.dom, 'clock',
+                        offset='variable',
+                        adjustment=str(self.conf.get('timeOffset', 0)))
+        appendElement(clock, 'timer', name='rtc', tickpolicy='catchup')
+        appendElement(clock, 'timer', name='pit', tickpolicy='delay')
         if self.arch == caps.Architecture.X86_64:
-            m.appendChildWithArgs('timer', name='hpet', present='no')
-
-        self.dom.appendChild(m)
+            appendElement(clock, 'timer', name='hpet', present='no')
 
     def appendOs(self):
         """
@@ -197,35 +163,33 @@
         </os>
         """
 
-        oselem = XMLElement('os')
-        self.dom.appendChild(oselem)
+        os = appendElement(self.dom, 'os')
 
         DEFAULT_MACHINES = {caps.Architecture.X86_64: 'pc',
                             caps.Architecture.PPC64: 'pseries'}
 
         machine = self.conf.get('emulatedMachine', DEFAULT_MACHINES[self.arch])
 
-        oselem.appendChildWithArgs('type', text='hvm', arch=self.arch,
-                                   machine=machine)
+        appendElement(os, 'type', arch=self.arch, machine=machine).text='hvm'
 
         qemu2libvirtBoot = {'a': 'fd', 'c': 'hd', 'd': 'cdrom', 'n': 'network'}
         for c in self.conf.get('boot', ''):
-            oselem.appendChildWithArgs('boot', dev=qemu2libvirtBoot[c])
+            appendElement(os, 'boot', dev=qemu2libvirtBoot[c])
 
         if self.conf.get('initrd'):
-            oselem.appendChildWithArgs('initrd', text=self.conf['initrd'])
+            appendElement(os, 'initrd').text=self.conf['initrd']
 
         if self.conf.get('kernel'):
-            oselem.appendChildWithArgs('kernel', text=self.conf['kernel'])
+            appendElement(os, 'kernel').text=self.conf['kernel']
 
         if self.conf.get('kernelArgs'):
-            oselem.appendChildWithArgs('cmdline', text=self.conf['kernelArgs'])
+            appendElement(os, 'cmdline').text=self.conf['kernelArgs']
 
         if self.arch == caps.Architecture.X86_64:
-            oselem.appendChildWithArgs('smbios', mode='sysinfo')
+            appendElement(os, 'smbios', mode='sysinfo')
 
         if utils.tobool(self.conf.get('bootMenuEnable', False)):
-            oselem.appendChildWithArgs('bootmenu', enable='yes')
+            appendElement(os, 'bootmenu', enable='yes')
 
     def appendSysinfo(self, osname, osversion, serialNumber):
         """
@@ -246,20 +210,15 @@
         </sysinfo>
         """
 
-        sysinfoelem = XMLElement('sysinfo', type='smbios')
-        self.dom.appendChild(sysinfoelem)
+        sysinfo = appendElement(self.dom, 'sysinfo', type='smbios')
+        syselem = appendElement(sysinfo, 'system')
 
-        syselem = XMLElement('system')
-        sysinfoelem.appendChild(syselem)
-
-        def appendEntry(k, v):
-            syselem.appendChildWithArgs('entry', text=v, name=k)
-
-        appendEntry('manufacturer', constants.SMBIOS_MANUFACTURER)
-        appendEntry('product', osname)
-        appendEntry('version', osversion)
-        appendEntry('serial', serialNumber)
-        appendEntry('uuid', self.conf['vmId'])
+        appendElement(syselem, 'entry', name='manufacturer').text=\
+                constants.SMBIOS_MANUFACTURER
+        appendElement(syselem, 'entry', name='product').text=osname
+        appendElement(syselem, 'entry', name='version').text=osversion
+        appendElement(syselem, 'entry', name='serial').text=serialNumber
+        appendElement(syselem, 'entry', name='uuid').text=self.conf['vmId']
 
     def appendFeatures(self):
         """
@@ -272,8 +231,8 @@
         """
 
         if utils.tobool(self.conf.get('acpiEnable', 'true')):
-            features = self.dom.appendChildWithArgs('features')
-            features.appendChildWithArgs('acpi')
+            features = appendElement(self.dom, 'features')
+            appendElement(features, 'acpi')
 
     def appendCpu(self):
         """
@@ -287,20 +246,20 @@
         </cpu>
         """
 
-        cpu = XMLElement('cpu')
+        cpu = ET.Element('cpu')
 
-        if self.arch in (caps.Architecture.X86_64):
-            cpu.setAttrs(match='exact')
+        if self.arch == caps.Architecture.X86_64:
+            cpu.set('match', 'exact')
 
             features = self.conf.get('cpuType', 'qemu64').split(',')
             model = features[0]
 
             if model == 'hostPassthrough':
-                cpu.setAttrs(mode='host-passthrough')
+                cpu.set('mode', 'host-passthrough')
             elif model == 'hostModel':
-                cpu.setAttrs(mode='host-model')
+                cpu.set('mode', 'host-model')
             else:
-                cpu.appendChildWithArgs('model', text=model)
+                appendElement(cpu, 'model').text=model
 
                 # This hack is for backward compatibility as the libvirt
                 # does not allow 'qemu64' guest on intel hardware
@@ -317,39 +276,39 @@
                         featureAttrs['policy'] = 'require'
                     elif feature[0] == '-':
                         featureAttrs['policy'] = 'disable'
-                    cpu.appendChildWithArgs('feature', **featureAttrs)
+
+                    appendElement(cpu, 'feature', featureAttrs)
 
         if ('smpCoresPerSocket' in self.conf or
                 'smpThreadsPerCore' in self.conf):
             maxVCpus = int(self._getMaxVCpus())
             cores = int(self.conf.get('smpCoresPerSocket', '1'))
             threads = int(self.conf.get('smpThreadsPerCore', '1'))
-            cpu.appendChildWithArgs('topology',
-                                    sockets=str(maxVCpus / cores / threads),
-                                    cores=str(cores), threads=str(threads))
+            appendElement(cpu, 'topology',
+                            sockets=str(maxVCpus / cores / threads),
+                            cores=str(cores), threads=str(threads))
 
         # CPU-pinning support
         # see http://www.ovirt.org/wiki/Features/Design/cpu-pinning
         if 'cpuPinning' in self.conf:
-            cputune = XMLElement('cputune')
+            cputune = appendElement(self.dom, 'cputune')
             cpuPinning = self.conf.get('cpuPinning')
-            for cpuPin in cpuPinning.keys():
-                cputune.appendChildWithArgs('vcpupin', vcpu=cpuPin,
-                                            cpuset=cpuPinning[cpuPin])
-            self.dom.appendChild(cputune)
+            for vcpu, cpuset in cpuPinning.items():
+                appendElement(cputune, 'vcpupin',
+                                vcpu=vcpu, cpuset=cpuset)
 
         # Guest numa topology support
         # see http://www.ovirt.org/Features/NUMA_and_Virtual_NUMA
         if 'guestNumaNodes' in self.conf:
-            numa = XMLElement('numa')
+            numa = appendElement(cpu, 'numa')
             guestNumaNodes = self.conf.get('guestNumaNodes')
             for vmCell in guestNumaNodes:
-                numa.appendChildWithArgs('cell',
-                                         cpus=vmCell['cpus'],
-                                         memory=str(vmCell['memory']))
-            cpu.appendChild(numa)
+                appendElement(numa, 'cell',
+                                cpus=vmCell['cpus'],
+                                memory=str(vmCell['memory']))
 
-        self.dom.appendChild(cpu)
+        self.dom.append(cpu)
+
 
     # Guest numatune support
     def appendNumaTune(self):
@@ -363,12 +322,11 @@
 
         if 'numaTune' in self.conf:
             numaTune = self.conf.get('numaTune')
-            if 'nodeset' in numaTune.keys():
+            if 'nodeset' in numaTune:
                 mode = numaTune.get('mode', 'strict')
-                numatune = XMLElement('numatune')
-                numatune.appendChildWithArgs('memory', mode=mode,
-                                             nodeset=numaTune['nodeset'])
-                self.dom.appendChild(numatune)
+                numatune = appendElement(self.dom, 'numatune')
+                appendElement(numatune, 'memory',
+                                mode=mode, nodeset=numaTune['nodeset'])
 
     def _appendAgentDevice(self, path, name):
         """
@@ -377,10 +335,9 @@
              <source mode='bind' path='/tmp/socket'/>
           </channel>
         """
-        channel = XMLElement('channel', type='unix')
-        channel.appendChildWithArgs('target', type='virtio', name=name)
-        channel.appendChildWithArgs('source', mode='bind', path=path)
-        self._devices.appendChild(channel)
+        channel = appendElement(self._devices, 'channel', type='unix')
+        appendElement(channel, 'target', type='virtio', name=name)
+        appendElement(channel, 'source', mode='bind', path=path)
 
     def appendInput(self):
         """
@@ -397,7 +354,7 @@
                 mouseBus = 'ps2'
 
             inputAttrs = {'type': 'mouse', 'bus': mouseBus}
-        self._devices.appendChildWithArgs('input', **inputAttrs)
+        appendElement(self._devices, 'input', inputAttrs)
 
     def appendKeyboardDevice(self):
         """
@@ -409,10 +366,9 @@
                 <qemu:arg value='keyboard'/>
             </qemu:commandline>
         """
-        commandLine = XMLElement('qemu:commandline')
-        commandLine.appendChildWithArgs('qemu:arg', value='-usbdevice')
-        commandLine.appendChildWithArgs('qemu:arg', value='keyboard')
-        self.dom.appendChild(commandLine)
+        commandLine = appendElement(self.dom, 'qemu:commandline')
+        appendElement(commandLine, 'qemu:arg', value='-usbdevice')
+        appendElement(commandLine, 'qemu:arg', value='keyboard')
 
     def appendGraphics(self):
         """
@@ -442,43 +398,56 @@
             graphicsAttrs['passwd'] = '*****'
             graphicsAttrs['passwdValidTo'] = '1970-01-01T00:00:01'
 
-        graphics = XMLElement('graphics', **graphicsAttrs)
+        graphics = appendElement(self._devices, 'graphics', graphicsAttrs)
 
         if 'qxl' in self.conf['display']:
             if self.conf.get('spiceSecureChannels'):
                 for channel in self.conf['spiceSecureChannels'].split(','):
-                    graphics.appendChildWithArgs('channel', name=channel[1:],
-                                                 mode='secure')
+                    appendElement(graphics, 'channel',
+                                    name=channel[1:], mode='secure')
 
-            vmc = XMLElement('channel', type='spicevmc')
-            vmc.appendChildWithArgs('target', type='virtio',
-                                    name='com.redhat.spice.0')
-            self._devices.appendChild(vmc)
+            vmc = appendElement(self._devices, 'channel', type='spicevmc')
+            appendElement(vmc, 'target',
+                            type='virtio', name='com.redhat.spice.0')
 
         if self.conf.get('displayNetwork'):
-            graphics.appendChildWithArgs('listen', type='network',
-                                         network=netinfo.LIBVIRT_NET_PREFIX +
-                                         self.conf.get('displayNetwork'))
+            appendElement(graphics, 'listen',
+                            type='network',
+                            network=netinfo.LIBVIRT_NET_PREFIX +
+                                self.conf.get('displayNetwork'))
         else:
-            graphics.setAttrs(listen='0')
-
-        self._devices.appendChild(graphics)
+            graphics.set('listen', '0')
 
     def appendEmulator(self):
-        emulatorPath = '/usr/bin/qemu-system-' + self.arch
+        appendElement(self._devices, 'emulator').text=\
+                '/usr/bin/qemu-system-%s' % self.arch
 
-        emulator = XMLElement('emulator', text=emulatorPath)
-
-        self._devices.appendChild(emulator)
+    def appendDevice(self, dev):
+        self._devices.append(dev.getXML())
 
     def toxml(self):
-        return self.doc.toprettyxml(encoding='utf-8')
+        # FIXME
+        return '<?xml version="1.0" encoding="utf-8"?>' + \
+            tostring(self.dom)
 
     def _getSmp(self):
         return self.conf.get('smp', '1')
 
     def _getMaxVCpus(self):
         return self.conf.get('maxVCpus', self._getSmp())
+
+
+def tostring(element):
+    return ET.tostring(element, encoding='utf-8', method='xml')
+
+
+def setattrs(element, attrs=None, **kwargs):
+    if attrs is None:
+        attrs = {}
+    attrs.update(kwargs)
+    for attrName, attrValue in attrs.items():
+        element.set(attrName, attrValue)
+    return element
 
 
 class XMLRepresentable(object):
@@ -488,10 +457,10 @@
         Create domxml device element according to passed in params
         """
         elemAttrs = {}
-        element = XMLElement(elemType)
+        element = ET.Element(elemType)
 
         if deviceType:
-            elemAttrs['type'] = deviceType
+            element.set('type', deviceType)
 
         for attrName in attributes:
             if not hasattr(self, attrName):
@@ -499,11 +468,10 @@
 
             attr = getattr(self, attrName)
             if isinstance(attr, dict):
-                element.appendChildWithArgs(attrName, **attr)
+                ET.SubElement(element, attrName, attr)
             else:
-                elemAttrs[attrName] = attr
+                element.set(attrName, attr)
 
-        element.setAttrs(**elemAttrs)
         return element
 
     def getXML(self):
@@ -512,43 +480,5 @@
         """
         return self.createXmlElem(self.type, self.device, ['address'])
 
-
-# A little unrelated hack to make xml.dom.minidom.Document.toprettyxml()
-# not wrap Text node with whitespace.
-# until http://bugs.python.org/issue4147 is accepted
-def __hacked_writexml(self, writer, indent="", addindent="", newl=""):
-
-    # copied from xml.dom.minidom.Element.writexml and hacked not to wrap Text
-    # nodes with whitespace.
-
-    # indent = current indentation
-    # addindent = indentation to add to higher levels
-    # newl = newline string
-    writer.write(indent + "<" + self.tagName)
-
-    attrs = self._get_attributes()
-    a_names = attrs.keys()
-    a_names.sort()
-
-    for a_name in a_names:
-        writer.write(" %s=\"" % a_name)
-        # _write_data(writer, attrs[a_name].value) # replaced
-        xml.dom.minidom._write_data(writer, attrs[a_name].value)
-        writer.write("\"")
-    if self.childNodes:
-        # added special handling of Text nodes
-        if (len(self.childNodes) == 1 and
-                isinstance(self.childNodes[0], xml.dom.minidom.Text)):
-            writer.write(">")
-            self.childNodes[0].writexml(writer)
-            writer.write("</%s>%s" % (self.tagName, newl))
-        else:
-            writer.write(">%s" % (newl))
-            for node in self.childNodes:
-                node.writexml(writer, indent + addindent, addindent, newl)
-            writer.write("%s</%s>%s" % (indent, self.tagName, newl))
-    else:
-        writer.write("/>%s" % (newl))
-
-
-xml.dom.minidom.Element.writexml = __hacked_writexml
+    def toXML(self):
+        return tostring(self.getXML())


-- 
To view, visit http://gerrit.ovirt.org/26856
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7874026acf52b869b8329f433d5833530e0d02e0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>


More information about the vdsm-patches mailing list