Change in vdsm[ovirt-3.5]: Live Merge: Get volume chain for multiple drives

alitke at redhat.com alitke at redhat.com
Tue Sep 2 17:20:43 UTC 2014


Hello Federico Simoncelli, Dan Kenigsberg, Francesco Romani,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/32322

to review the following change.

Change subject: Live Merge: Get volume chain for multiple drives
......................................................................

Live Merge: Get volume chain for multiple drives

A future patch wants to get volume chain information for all drives in
one call.  Rather than getting the domXML once per drive, allow
_driveGetActualVolumeChain to accept a list of drives and parse the
information all at once.

Change-Id: I93d5a641b0814e3764c70bea8a6c1910a821adcc
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1109920
Signed-off-by: Adam Litke <alitke at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/31366
Reviewed-by: Francesco Romani <fromani at redhat.com>
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
Reviewed-by: Federico Simoncelli <fsimonce at redhat.com>
---
M vdsm/virt/vm.py
1 file changed, 42 insertions(+), 34 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/22/32322/1

diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 63a2d00..0811723 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -5668,8 +5668,8 @@
             return errCode['imageErr']
 
         # Check that libvirt exposes full volume chain information
-        chain = self._driveGetActualVolumeChain(drive)
-        if chain is None:
+        chains = self._driveGetActualVolumeChain([drive])
+        if drive['alias'] not in chains:
             self.log.error("merge: libvirt does not support volume chain "
                            "monitoring.  Unable to perform live merge.")
             return errCode['mergeErr']
@@ -5745,53 +5745,59 @@
 
         return {'status': doneCode}
 
-    def _lookupDeviceXMLByAlias(self, targetAlias):
-        domXML = self._getUnderlyingVmInfo()
-        devices = _domParseStr(domXML).childNodes[0]. \
-            getElementsByTagName('devices')[0]
+    def _diskXMLGetVolumeChainInfo(self, diskXML, drive):
+        def find_element_by_name(doc, name):
+            for child in doc.childNodes:
+                if child.nodeName == name:
+                    return child
+            return None
 
-        for deviceXML in devices.childNodes:
-            if deviceXML.nodeType != Node.ELEMENT_NODE:
-                continue
-
-            aliasElement = deviceXML.getElementsByTagName('alias')
-            if aliasElement:
-                alias = aliasElement[0].getAttribute('name')
-
-                if alias == targetAlias:
-                    return deviceXML
-        return None
-
-    def _driveGetActualVolumeChain(self, drive):
         def pathToVolID(drive, path):
             for vol in drive.volumeChain:
                 if os.path.realpath(vol['path']) == os.path.realpath(path):
                     return vol['volumeID']
             raise LookupError("Unable to find VolumeID for path '%s'", path)
 
-        def findElement(doc, name):
-            for child in doc.childNodes:
-                if child.nodeName == name:
-                    return child
-            return None
-
-        diskXML = self._lookupDeviceXMLByAlias(drive['alias'])
-        if not diskXML:
-            return None
         volChain = []
         while True:
-            sourceXML = findElement(diskXML, 'source')
+            sourceXML = find_element_by_name(diskXML, 'source')
             if not sourceXML:
                 break
             sourceAttr = ('file', 'dev')[drive.blockDev]
             path = sourceXML.getAttribute(sourceAttr)
             entry = VolumeChainEntry(pathToVolID(drive, path), path)
-            volChain.insert(0, entry)
-            bsXML = findElement(diskXML, 'backingStore')
+            bsXML = find_element_by_name(diskXML, 'backingStore')
             if not bsXML:
+                self.log.warning("<backingStore/> missing from backing "
+                                 "chain for drive %s", drive.name)
                 break
             diskXML = bsXML
-        return volChain
+            volChain.insert(0, entry)
+        return volChain or None
+
+    def _driveGetActualVolumeChain(self, drives):
+        def lookupDeviceXMLByAlias(domXML, targetAlias):
+            domObj = xml.dom.minidom.parseString(domXML)
+            devices = domObj.childNodes[0].getElementsByTagName('devices')[0]
+            for deviceXML in devices.childNodes:
+                if deviceXML.nodeType == xml.dom.Node.ELEMENT_NODE:
+                    aliasElement = deviceXML.getElementsByTagName('alias')
+                    if aliasElement:
+                        alias = aliasElement[0].getAttribute('name')
+                        if alias == targetAlias:
+                            return deviceXML
+            raise LookupError("Unable to find matching XML for device %s",
+                              targetAlias)
+
+        ret = {}
+        domXML = self._getUnderlyingVmInfo()
+        for drive in drives:
+            alias = drive['alias']
+            diskXML = lookupDeviceXMLByAlias(domXML, alias)
+            volChain = self._diskXMLGetVolumeChainInfo(diskXML, drive)
+            if volChain:
+                ret[alias] = volChain
+        return ret
 
     def _syncVolumeChain(self, drive):
         def getVolumeInfo(device, volumeID):
@@ -5805,8 +5811,10 @@
             return
 
         curVols = [x['volumeID'] for x in drive.volumeChain]
-        chain = self._driveGetActualVolumeChain(drive)
-        if chain is None:
+        chains = self._driveGetActualVolumeChain([drive])
+        try:
+            chain = chains[drive['alias']]
+        except KeyError:
             self.log.debug("Unable to determine volume chain. Skipping volume "
                            "chain synchronization for drive %s", drive.name)
             return


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I93d5a641b0814e3764c70bea8a6c1910a821adcc
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>


More information about the vdsm-patches mailing list