Francesco Romani has uploaded a new change for review. Change subject: virt: move all_channels into DomainDescriptor ......................................................................
virt: move all_channels into DomainDescriptor before the introduction of the DomainDescriptor class, we had a bunch of functions/methods who repeteadly parsed the VM XML Domain using the slow minidom module. The introduction of the DomainDescriptor opens the gate for both more efficient and cleaner processing. DomainDescriptor should provide methods to return high-level objects, not low-level ones, e.g, XML document nodes, which are inevitabily tied to the underlying xml library being used. The first step in this (long) journey is to move vmxml.all_channels into the DomainDescriptor. This allow us to have both nicer code in vm.Vm and to save us one unneeded XML parsing. Change-Id: I870cce435b2e26ff33cbd52f2421c1f3ce661707 Signed-off-by: Francesco Romani <[email protected]> --- M tests/vmXmlTests.py M vdsm/virt/domain_descriptor.py M vdsm/virt/vm.py M vdsm/virt/vmxml.py 4 files changed, 45 insertions(+), 40 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/15/34415/1 diff --git a/tests/vmXmlTests.py b/tests/vmXmlTests.py index 17ada42..2e73d6f 100644 --- a/tests/vmXmlTests.py +++ b/tests/vmXmlTests.py @@ -18,6 +18,7 @@ # Refer to the README and COPYING files for full details of the license # +from virt import domain_descriptor from virt import vm from virt import vmxml import caps @@ -32,29 +33,7 @@ from vmTestsData import CONF_TO_DOMXML_NO_VDSM -@expandPermutations -class TestVmXmlFunctions(TestCaseBase): - - @permutations([[caps.Architecture.X86_64], - [caps.Architecture.PPC64]]) - def test_has_channel(self, arch): - for _, dom_xml in self._build_domain_xml(arch): - self.assertEqual(True, vmxml.has_channel( - dom_xml, vm._VMCHANNEL_DEVICE_NAME)) - - @permutations([[caps.Architecture.X86_64], [caps.Architecture.PPC64]]) - def test_all_channels_vdsm_domain(self, arch): - for _, dom_xml in self._build_domain_xml(arch): - channels = list(vmxml.all_channels(dom_xml)) - self.assertTrue(len(channels) >= len(vm._AGENT_CHANNEL_DEVICES)) - for name, path in channels: - self.assertIn(name, vm._AGENT_CHANNEL_DEVICES) - - def test_all_channels_extra_domain(self): - for conf, raw_xml in CONF_TO_DOMXML_NO_VDSM: - dom_xml = raw_xml % conf - self.assertNotEquals(sorted(vmxml.all_channels(dom_xml)), - sorted(vm._AGENT_CHANNEL_DEVICES)) +class VmXmlTestCase(TestCaseBase): _CONFS = { caps.Architecture.X86_64: CONF_TO_DOMXML_X86_64, @@ -65,3 +44,33 @@ for conf, rawXml in self._CONFS[arch]: domXml = rawXml % conf yield fake.Domain(domXml, vmId=conf['vmId']), domXml + + +@expandPermutations +class TestVmXmlFunctions(VmXmlTestCase): + + @permutations([[caps.Architecture.X86_64], + [caps.Architecture.PPC64]]) + def test_has_channel(self, arch): + for _, dom_xml in self._build_domain_xml(arch): + self.assertEqual(True, vmxml.has_channel( + dom_xml, vm._VMCHANNEL_DEVICE_NAME)) + + +@expandPermutations +class TestDomainDescriptor(VmXmlTestCase): + + @permutations([[caps.Architecture.X86_64], [caps.Architecture.PPC64]]) + def test_all_channels_vdsm_domain(self, arch): + for _, dom_xml in self._build_domain_xml(arch): + dom = domain_descriptor.DomainDescriptor(dom_xml) + channels = list(dom.all_channels()) + self.assertTrue(len(channels) >= len(vm._AGENT_CHANNEL_DEVICES)) + for name, path in channels: + self.assertIn(name, vm._AGENT_CHANNEL_DEVICES) + + def test_all_channels_extra_domain(self): + for conf, raw_xml in CONF_TO_DOMXML_NO_VDSM: + dom = domain_descriptor.DomainDescriptor(raw_xml % conf) + self.assertNotEquals(sorted(dom.all_channels()), + sorted(vm._AGENT_CHANNEL_DEVICES)) diff --git a/vdsm/virt/domain_descriptor.py b/vdsm/virt/domain_descriptor.py index 3100a25..a5b2956 100644 --- a/vdsm/virt/domain_descriptor.py +++ b/vdsm/virt/domain_descriptor.py @@ -52,6 +52,18 @@ def devicesHash(self): return self._devicesHash + def all_channels(self): + for channel in self.getDeviceElements('channel'): + try: + name = channel.getElementsByTagName('target')[0].\ + getAttribute('name') + path = channel.getElementsByTagName('source')[0].\ + getAttribute('path') + except IndexError: + continue + else: + yield name, path + def _firstElementByTagName(self, tagName): elements = self._dom.childNodes[0].getElementsByTagName(tagName) return elements[0] if elements else None diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index 2992674..59cff66 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -2703,7 +2703,7 @@ This is necessary to prevent incoming migrations, restoring of VMs and the upgrade of VDSM with running VMs to fail on this. """ - for name, path in vmxml.all_channels(self._domain.xml): + for name, path in self._domain.all_channels(): if name not in _AGENT_CHANNEL_DEVICES: continue diff --git a/vdsm/virt/vmxml.py b/vdsm/virt/vmxml.py index eb07ba9..f8b546d 100644 --- a/vdsm/virt/vmxml.py +++ b/vdsm/virt/vmxml.py @@ -44,22 +44,6 @@ return False -def all_channels(domXML): - domObj = xml.dom.minidom.parseString(domXML) - for channel in domObj.childNodes[0]. \ - getElementsByTagName('devices')[0]. \ - getElementsByTagName('channel'): - try: - name = channel.getElementsByTagName('target')[0].\ - getAttribute('name') - path = channel.getElementsByTagName('source')[0].\ - getAttribute('path') - except IndexError: - continue - else: - yield name, path - - def all_devices(domXML): domObj = xml.dom.minidom.parseString(domXML) devices = domObj.childNodes[0].getElementsByTagName('devices')[0] -- To view, visit http://gerrit.ovirt.org/34415 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I870cce435b2e26ff33cbd52f2421c1f3ce661707 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
