Change in vdsm[master]: vdsm: Refactoring of retrieving device info from xml

vfeenstr at redhat.com vfeenstr at redhat.com
Tue Aug 6 07:03:38 UTC 2013


Vinzenz Feenstra has uploaded a new change for review.

Change subject: vdsm: Refactoring of retrieving device info from xml
......................................................................

vdsm: Refactoring of retrieving device info from xml

Reworked a bit the retrieval of device info from the libvirt domain xml.
Now VDSM won't parse the code in lastXmlDesc every time and the retrieval
of elements from the domain xml has been a bit abstracted.
Additionally the retrieval of an alias has been moved into a separate
function call to make the readability a bit better.

Change-Id: I7e106b2f2d3f4160d4e882f1a2880cb1b52fbb22
Signed-off-by: Vinzenz Feenstra <vfeenstr at redhat.com>
---
M vdsm/vm.py
1 file changed, 63 insertions(+), 76 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/94/17694/1

diff --git a/vdsm/vm.py b/vdsm/vm.py
index dc52909..e51050e 100644
--- a/vdsm/vm.py
+++ b/vdsm/vm.py
@@ -1698,6 +1698,7 @@
         self._guestSocketFile = self._makeChannelPath(_VMCHANNEL_DEVICE_NAME)
         self._qemuguestSocketFile = self._makeChannelPath(_QEMU_GA_DEVICE_NAME)
         self._lastXMLDesc = '<domain><uuid>%s</uuid></domain>' % self.id
+        self._lastParsedXmlDesc = _domParseStr(self._lastXMLDesc)
         self._devXmlHash = '0'
         self._released = False
         self._releaseLock = threading.Lock()
@@ -2722,24 +2723,30 @@
         self._guestCpuRunning = (self._dom.info()[0] ==
                                  libvirt.VIR_DOMAIN_RUNNING)
 
+    def _getDevicesXml(self, parsedXml=None):
+        parsedXml = parsedXml or self._lastParsedXmlDesc
+        return parsedXml.childNodes[0].getElementsByTagName('devices')[0]
+
     def _getUnderlyingVmDevicesInfo(self):
         """
         Obtain underlying vm's devices info from libvirt.
         """
-        self._getUnderlyingNetworkInterfaceInfo()
-        self._getUnderlyingDriveInfo()
-        self._getUnderlyingDisplayPort()
-        self._getUnderlyingSoundDeviceInfo()
-        self._getUnderlyingVideoDeviceInfo()
-        self._getUnderlyingControllerDeviceInfo()
-        self._getUnderlyingBalloonDeviceInfo()
-        self._getUnderlyingWatchdogDeviceInfo()
-        self._getUnderlyingSmartcardDeviceInfo()
-        self._getUnderlyingConsoleDeviceInfo()
+        devicesXml = self._getDevicesXml(parsedXml=self._lastParsedXmlDesc)
+        self._getUnderlyingNetworkInterfaceInfo(devicesXml=devicesXml)
+        self._getUnderlyingDriveInfo(devicesXml=devicesXml)
+        self._getUnderlyingDisplayPort(xml=self._lastParsedXmlDesc)
+        self._getUnderlyingSoundDeviceInfo(devicesXml=devicesXml)
+        self._getUnderlyingVideoDeviceInfo(devicesXml=devicesXml)
+        self._getUnderlyingControllerDeviceInfo(devicesXml=devicesXml)
+        self._getUnderlyingBalloonDeviceInfo(devicesXml=devicesXml)
+        self._getUnderlyingWatchdogDeviceInfo(devicesXml=devicesXml)
+        self._getUnderlyingSmartcardDeviceInfo(devicesXml=devicesXml)
+        self._getUnderlyingConsoleDeviceInfo(devicesXml=devicesXml)
         # Obtain info of all unknown devices. Must be last!
-        self._getUnderlyingUnknownDeviceInfo()
+        self._getUnderlyingUnknownDeviceInfo(devicesXml=devicesXml)
+        self._updateAgentChannels(devicesXml=devicesXml)
 
-    def _updateAgentChannels(self):
+    def _updateAgentChannels(self, devicesXml):
         """
         We moved the naming of guest agent channel sockets. To keep backwards
         compatability we need to make symlinks from the old channel sockets, to
@@ -2747,9 +2754,7 @@
         This is necessary to prevent incoming migrations, restoring of VMs and
         the upgrade of VDSM with running VMs to fail on this.
         """
-        agentChannelXml = _domParseStr(self._lastXMLDesc).childNodes[0]. \
-            getElementsByTagName('devices')[0]. \
-            getElementsByTagName('channel')
+        agentChannelXml = devicesXml.getElementsByTagName('channel')
         for channel in agentChannelXml:
             try:
                 name = channel.getElementsByTagName('target')[0].\
@@ -2781,7 +2786,6 @@
 
         self._getUnderlyingVmInfo()
         self._getUnderlyingVmDevicesInfo()
-        self._updateAgentChannels()
 
         #Currently there is no protection agains mirroring a network twice,
         for nic in self._devices[NIC_DEVICES]:
@@ -2937,9 +2941,8 @@
         or revert to snapshot.
         """
         parsedSrcDomXML = _domParseStr(srcDomXML)
-
-        allDiskDeviceXmlElements = parsedSrcDomXML.childNodes[0]. \
-            getElementsByTagName('devices')[0].getElementsByTagName('disk')
+        devicesXml = self._getDevicesXml(parsedXml=parsedSrcDomXML)
+        allDiskDeviceXmlElements = devicesXml.getElementsByTagName('disk')
 
         snappableDiskDeviceXmlElements = \
             _filterSnappableDiskDevices(allDiskDeviceXmlElements)
@@ -3008,7 +3011,8 @@
             with self._confLock:
                 self.conf['devices'].append(nicParams)
             self.saveState()
-            self._getUnderlyingNetworkInterfaceInfo()
+            self._getUnderlyingNetworkInterfaceInfo(
+                devicesXml=self._getDevicesXml())
             hooks.after_nic_hotplug(nicXml, self.conf,
                                     params=customProps)
 
@@ -3264,7 +3268,7 @@
             with self._confLock:
                 self.conf['devices'].append(diskParams)
             self.saveState()
-            self._getUnderlyingDriveInfo()
+            self._getUnderlyingDriveInfo(devicesXml=self._getDevicesXml())
             hooks.after_disk_hotplug(driveXml, self.conf,
                                      params=customProps)
 
@@ -4181,8 +4185,8 @@
 
     def _getUnderlyingVmInfo(self):
         self._lastXMLDesc = self._dom.XMLDesc(0)
-        devxml = _domParseStr(self._lastXMLDesc).childNodes[0]. \
-            getElementsByTagName('devices')[0]
+        self._lastParsedXmlDesc = _domParseStr(self._lastXMLDesc)
+        devxml = self._getDevicesXml()
         self._devXmlHash = str(hash(devxml.toxml()))
 
         return self._lastXMLDesc
@@ -4331,6 +4335,9 @@
             self.saveState()
             return {'status': doneCode}
 
+    def _getUnderlyingDeviceAliasName(self, devXml):
+        return devXml.getElementsByTagName('alias')[0].getAttribute('name')
+
     def _getUnderlyingDeviceAddress(self, devXml):
         """
         Obtain device's address from libvirt
@@ -4347,7 +4354,7 @@
 
         return address
 
-    def _getUnderlyingUnknownDeviceInfo(self):
+    def _getUnderlyingUnknownDeviceInfo(self, devicesXml):
         """
         Obtain unknown devices info from libvirt.
 
@@ -4360,16 +4367,13 @@
                     return True
             return False
 
-        devsxml = _domParseStr(self._lastXMLDesc).childNodes[0]. \
-            getElementsByTagName('devices')[0]
-
-        for x in devsxml.childNodes:
+        for x in devicesXml.childNodes:
             # Ignore empty nodes and devices without address
             if (x.nodeName == '#text' or
                     not x.getElementsByTagName('address')):
                 continue
 
-            alias = x.getElementsByTagName('alias')[0].getAttribute('name')
+            alias = self._getUnderlyingDeviceAliasName(x)
             if not isKnownDevice(alias):
                 address = self._getUnderlyingDeviceAddress(x)
                 # I general case we assume that device has attribute 'type',
@@ -4381,18 +4385,16 @@
                           'address': address}
                 self.conf['devices'].append(newDev)
 
-    def _getUnderlyingControllerDeviceInfo(self):
+    def _getUnderlyingControllerDeviceInfo(self, devicesXml):
         """
         Obtain controller devices info from libvirt.
         """
-        ctrlsxml = _domParseStr(self._lastXMLDesc).childNodes[0]. \
-            getElementsByTagName('devices')[0]. \
-            getElementsByTagName('controller')
+        ctrlsxml = devicesXml.getElementsByTagName('controller')
         for x in ctrlsxml:
             # Ignore controller devices without address
             if not x.getElementsByTagName('address'):
                 continue
-            alias = x.getElementsByTagName('alias')[0].getAttribute('name')
+            alias = self._getUnderlyingDeviceAliasName(x)
             device = x.getAttribute('type')
             # Get model and index. Relevant for USB controllers.
             model = x.getAttribute('model')
@@ -4428,20 +4430,18 @@
                                              'address': address,
                                              'alias': alias})
 
-    def _getUnderlyingBalloonDeviceInfo(self):
+    def _getUnderlyingBalloonDeviceInfo(self, devicesXml):
         """
         Obtain balloon device info from libvirt.
         """
-        balloonxml = _domParseStr(self._lastXMLDesc).childNodes[0]. \
-            getElementsByTagName('devices')[0]. \
-            getElementsByTagName('memballoon')
+        balloonxml = devicesXml.getElementsByTagName('memballoon')
         for x in balloonxml:
             # Ignore balloon devices without address.
             if not x.getElementsByTagName('address'):
                 address = None
             else:
                 address = self._getUnderlyingDeviceAddress(x)
-            alias = x.getElementsByTagName('alias')[0].getAttribute('name')
+            alias = self._getUnderlyingDeviceAliasName(x)
 
             for dev in self._devices[BALLOON_DEVICES]:
                 if address and not hasattr(dev, 'address'):
@@ -4456,16 +4456,14 @@
                     if not dev.get('alias'):
                         dev['alias'] = alias
 
-    def _getUnderlyingConsoleDeviceInfo(self):
+    def _getUnderlyingConsoleDeviceInfo(self, devicesXml):
         """
         Obtain the alias for the console device from libvirt
         """
-        consolexml = _domParseStr(self._lastXMLDesc).childNodes[0].\
-            getElementsByTagName('devices')[0].\
-            getElementsByTagName('console')
+        consolexml = devicesXml.getElementsByTagName('console')
         for x in consolexml:
             # All we care about is the alias
-            alias = x.getElementsByTagName('alias')[0].getAttribute('name')
+            alias = self._getUnderlyingDeviceAliasName(x)
             for dev in self._devices[CONSOLE_DEVICES]:
                 if not hasattr(dev, 'alias'):
                     dev.alias = alias
@@ -4475,19 +4473,17 @@
                         not dev.get('alias'):
                     dev['alias'] = alias
 
-    def _getUnderlyingSmartcardDeviceInfo(self):
+    def _getUnderlyingSmartcardDeviceInfo(self, devicesXml):
         """
         Obtain smartcard device info from libvirt.
         """
-        smartcardxml = _domParseStr(self._lastXMLDesc).childNodes[0].\
-            getElementsByTagName('devices')[0].\
-            getElementsByTagName('smartcard')
+        smartcardxml = devicesXml.getElementsByTagName('smartcard')
         for x in smartcardxml:
             if not x.getElementsByTagName('address'):
                 continue
 
             address = self._getUnderlyingDeviceAddress(x)
-            alias = x.getElementsByTagName('alias')[0].getAttribute('name')
+            alias = self._getUnderlyingDeviceAliasName(x)
 
             for dev in self._devices[SMARTCARD_DEVICES]:
                 if not hasattr(dev, 'address'):
@@ -4500,19 +4496,17 @@
                     dev['address'] = address
                     dev['alias'] = alias
 
-    def _getUnderlyingWatchdogDeviceInfo(self):
+    def _getUnderlyingWatchdogDeviceInfo(self, devicesXml):
         """
         Obtain watchdog device info from libvirt.
         """
-        watchdogxml = _domParseStr(self._lastXMLDesc).childNodes[0]. \
-            getElementsByTagName('devices')[0]. \
-            getElementsByTagName('watchdog')
+        watchdogxml = devicesXml.getElementsByTagName('watchdog')
         for x in watchdogxml:
 
             # PCI watchdog has "address" different from ISA watchdog
             if x.getElementsByTagName('address'):
                 address = self._getUnderlyingDeviceAddress(x)
-                alias = x.getElementsByTagName('alias')[0].getAttribute('name')
+                alias = self._getUnderlyingDeviceAliasName(x)
 
                 for wd in self._devices[WATCHDOG_DEVICES]:
                     if not hasattr(wd, 'address') or not hasattr(wd, 'alias'):
@@ -4525,14 +4519,13 @@
                         dev['address'] = address
                         dev['alias'] = alias
 
-    def _getUnderlyingVideoDeviceInfo(self):
+    def _getUnderlyingVideoDeviceInfo(self, devicesXml):
         """
         Obtain video devices info from libvirt.
         """
-        videosxml = _domParseStr(self._lastXMLDesc).childNodes[0]. \
-            getElementsByTagName('devices')[0].getElementsByTagName('video')
+        videosxml = devicesXml.getElementsByTagName('video')
         for x in videosxml:
-            alias = x.getElementsByTagName('alias')[0].getAttribute('name')
+            alias = self._getUnderlyingDeviceAliasName(x)
             # Get video card address
             address = self._getUnderlyingDeviceAddress(x)
 
@@ -4553,14 +4546,13 @@
                     dev['alias'] = alias
                     break
 
-    def _getUnderlyingSoundDeviceInfo(self):
+    def _getUnderlyingSoundDeviceInfo(self, devicesXml):
         """
         Obtain sound devices info from libvirt.
         """
-        soundsxml = _domParseStr(self._lastXMLDesc).childNodes[0]. \
-            getElementsByTagName('devices')[0].getElementsByTagName('sound')
+        soundsxml = devicesXml.getElementsByTagName('sound')
         for x in soundsxml:
-            alias = x.getElementsByTagName('alias')[0].getAttribute('name')
+            alias = self._getUnderlyingDeviceAliasName(x)
             # Get sound card address
             address = self._getUnderlyingDeviceAddress(x)
 
@@ -4581,12 +4573,11 @@
                     dev['alias'] = alias
                     break
 
-    def _getUnderlyingDriveInfo(self):
+    def _getUnderlyingDriveInfo(self, devicesXml):
         """
         Obtain block devices info from libvirt.
         """
-        disksxml = _domParseStr(self._lastXMLDesc).childNodes[0]. \
-            getElementsByTagName('devices')[0].getElementsByTagName('disk')
+        disksxml = devicesXml.getElementsByTagName('disk')
         # FIXME!  We need to gather as much info as possible from the libvirt.
         # In the future we can return this real data to management instead of
         # vm's conf
@@ -4600,7 +4591,7 @@
 
             target = x.getElementsByTagName('target')
             name = target[0].getAttribute('dev') if target else ''
-            alias = x.getElementsByTagName('alias')[0].getAttribute('name')
+            alias = self._getUnderlyingDeviceAliasName(x)
             readonly = bool(x.getElementsByTagName('readonly'))
             boot = x.getElementsByTagName('boot')
             bootOrder = boot[0].getAttribute('order') if boot else ''
@@ -4646,12 +4637,11 @@
                     diskDev['bootOrder'] = bootOrder
                 self.conf['devices'].append(diskDev)
 
-    def _getUnderlyingDisplayPort(self):
+    def _getUnderlyingDisplayPort(self, xml):
         """
         Obtain display port info from libvirt.
         """
-        graphics = _domParseStr(self._lastXMLDesc).childNodes[0]. \
-            getElementsByTagName('graphics')[0]
+        graphics = xml.childNodes[0].getElementsByTagName('graphics')[0]
         port = graphics.getAttribute('port')
         if port:
             self.conf['displayPort'] = port
@@ -4659,18 +4649,16 @@
         if port:
             self.conf['displaySecurePort'] = port
 
-    def _getUnderlyingNetworkInterfaceInfo(self):
+    def _getUnderlyingNetworkInterfaceInfo(self, devicesXml):
         """
         Obtain network interface info from libvirt.
         """
         # TODO use xpath instead of parseString (here and elsewhere)
-        ifsxml = _domParseStr(self._lastXMLDesc).childNodes[0]. \
-            getElementsByTagName('devices')[0]. \
-            getElementsByTagName('interface')
+        ifsxml = devicesXml.getElementsByTagName('interface')
         for x in ifsxml:
             devType = x.getAttribute('type')
             mac = x.getElementsByTagName('mac')[0].getAttribute('address')
-            alias = x.getElementsByTagName('alias')[0].getAttribute('name')
+            alias = self._getUnderlyingDeviceAliasName(x)
             if devType == 'hostdev':
                 name = alias
                 model = 'passthrough'
@@ -4802,8 +4790,7 @@
                                    "during migration at destination host" %
                                    devType)
 
-        devices = _domParseStr(xml).childNodes[0]. \
-            getElementsByTagName('devices')[0]
+        devices = self._getDevicesXml(parsedXml=_domParseStr(xml))
 
         for deviceXML in devices.childNodes:
             if deviceXML.nodeType != Node.ELEMENT_NODE:


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7e106b2f2d3f4160d4e882f1a2880cb1b52fbb22
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr at redhat.com>


More information about the vdsm-patches mailing list