Francesco Romani has uploaded a new change for review. Change subject: vm: fix getVmPolicy return type. ......................................................................
vm: fix getVmPolicy return type. in case of error, getVmPolicy will return a VDSM API return value, which may confuse the caller. This patch makes getVmPolicy returns explicitely None on error, and let the caller handle this condition and bail out correctly. Change-Id: I4c1d79283ee2de4a5dcf54f3544ec346c68837e1 Signed-off-by: Francesco Romani <[email protected]> --- M tests/vmTests.py M vdsm/virt/vm.py 2 files changed, 34 insertions(+), 5 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/30/29930/1 diff --git a/tests/vmTests.py b/tests/vmTests.py index 7ee2eb8..2be5282 100644 --- a/tests/vmTests.py +++ b/tests/vmTests.py @@ -24,6 +24,7 @@ import re import shutil import tempfile +import xml.dom.minidom import xml.etree.ElementTree as ET import libvirt @@ -54,9 +55,16 @@ class FakeDomain: - def __init__(self, xml=''): + def __init__(self, xml='', virtError=libvirt.VIR_ERR_OK): self._xml = xml self.devXml = '' + self._virtError = virtError + + def _failIfRequested(self): + if self._virtError != libvirt.VIR_ERR_OK: + err = libvirt.libvirtError(defmsg='') + err.err = [self._virtError] + raise err def info(self): raise libvirt.libvirtError(defmsg='') @@ -71,7 +79,8 @@ return -1 def metadata(self, type, uri, flags): - return None + self._failIfRequested() + return '<qos></qos>' def schedulerParameters(self): return {'vcpu_quota': vm._NO_CPU_QUOTA, @@ -867,6 +876,24 @@ def testBuildCmdLinePPC64(self): self.assertBuildCmdLine(CONF_TO_DOMXML_PPC64) + def testGetVmPolicySucceded(self): + with FakeVM() as fake: + fake._dom = FakeDomain() + self.assertTrue(isinstance(fake._getVmPolicy(), + xml.dom.minidom.Element)) + + def testGetVmPolicyEmptyOnNoMetadata(self): + with FakeVM() as fake: + fake._dom = FakeDomain( + virtError=libvirt.VIR_ERR_NO_DOMAIN_METADATA) + self.assertTrue(isinstance(fake._getVmPolicy(), + xml.dom.minidom.Element)) + + def testGetVmPolicyFailOnNoDomain(self): + with FakeVM() as fake: + fake._dom = FakeDomain(virtError=libvirt.VIR_ERR_NO_DOMAIN) + self.assertEqual(fake._getVmPolicy(), None) + class FakeGuestAgent(object): def getGuestInfo(self): diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index f29c5f8..3fde1f7 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -3689,8 +3689,10 @@ # # Get the current QoS block - qos = self._getVmPolicy() metadata_modified = False + qos = self._getVmPolicy() + if qos is None: + return self._reportError(key='updateVmPolicyErr') # # Process provided properties, remove property after it is processed @@ -3752,8 +3754,8 @@ METADATA_VM_TUNE_URI, 0) except libvirt.libvirtError as e: if e.get_error_code() != libvirt.VIR_ERR_NO_DOMAIN_METADATA: - return self._reportException(key='updateVmPolicyErr', - msg=e.message) + self.log.exception("getVmPolicy failed") + return None metadata = xml.dom.minidom.parseString(metadata_xml) return metadata.getElementsByTagName("qos")[0] -- To view, visit http://gerrit.ovirt.org/29930 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4c1d79283ee2de4a5dcf54f3544ec346c68837e1 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
