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(a)redhat.com>
Reviewed-on:
http://gerrit.ovirt.org/31366
Reviewed-by: Francesco Romani <fromani(a)redhat.com>
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
Reviewed-by: Federico Simoncelli <fsimonce(a)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(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>