From 39f13fa706e79f1d56694d1c027b0f808ed5a68e Mon Sep 17 00:00:00 2001 From: liu-sheng Date: Fri, 24 Jan 2014 18:03:29 +0800 Subject: [PATCH] check domain state before inspecting nics/disks If there are shutoff instances,the ceilometer-inspector vill inspect faild,and record the tedious error string in log. The inspector of ceilometer should check the domain state before inspecting nics and disks,and record more conciser log message in log file. Change-Id: Id56bf75b47edf1f3b4ba77f40a48d83186d0afb6 Closes-bug: #1271454 --- ceilometer/compute/virt/libvirt/inspector.py | 18 +++++++-- .../compute/virt/libvirt/test_inspector.py | 38 +++++++++++++++++-- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/ceilometer/compute/virt/libvirt/inspector.py b/ceilometer/compute/virt/libvirt/inspector.py index ed453b7e9..95821c297 100644 --- a/ceilometer/compute/virt/libvirt/inspector.py +++ b/ceilometer/compute/virt/libvirt/inspector.py @@ -107,11 +107,17 @@ class LibvirtInspector(virt_inspector.Inspector): def inspect_cpus(self, instance_name): domain = self._lookup_by_name(instance_name) - (_, _, _, num_cpu, cpu_time) = domain.info() + (__, __, __, num_cpu, cpu_time) = domain.info() return virt_inspector.CPUStats(number=num_cpu, time=cpu_time) def inspect_vnics(self, instance_name): domain = self._lookup_by_name(instance_name) + (state, __, __, __, __) = domain.info() + if state == libvirt.VIR_DOMAIN_SHUTOFF: + LOG.warn(_('Failed to inspect vnics of %(instance_name)s, ' + 'domain is in state of SHUTOFF'), + {'instance_name': instance_name}) + return tree = etree.fromstring(domain.XMLDesc(0)) for iface in tree.findall('devices/interface'): target = iface.find('target') @@ -132,8 +138,8 @@ class LibvirtInspector(virt_inspector.Inspector): for p in iface.findall('filterref/parameter')) interface = virt_inspector.Interface(name=name, mac=mac_address, fref=fref, parameters=params) - rx_bytes, rx_packets, _, _, \ - tx_bytes, tx_packets, _, _ = domain.interfaceStats(name) + rx_bytes, rx_packets, __, __, \ + tx_bytes, tx_packets, __, __ = domain.interfaceStats(name) stats = virt_inspector.InterfaceStats(rx_bytes=rx_bytes, rx_packets=rx_packets, tx_bytes=tx_bytes, @@ -142,6 +148,12 @@ class LibvirtInspector(virt_inspector.Inspector): def inspect_disks(self, instance_name): domain = self._lookup_by_name(instance_name) + (state, __, __, __, __) = domain.info() + if state == libvirt.VIR_DOMAIN_SHUTOFF: + LOG.warn(_('Failed to inspect disks of %(instance_name)s, ' + 'domain is in state of SHUTOFF'), + {'instance_name': instance_name}) + return tree = etree.fromstring(domain.XMLDesc(0)) for device in filter( bool, diff --git a/ceilometer/tests/compute/virt/libvirt/test_inspector.py b/ceilometer/tests/compute/virt/libvirt/test_inspector.py index 55fc55567..83dcb5501 100644 --- a/ceilometer/tests/compute/virt/libvirt/test_inspector.py +++ b/ceilometer/tests/compute/virt/libvirt/test_inspector.py @@ -36,6 +36,8 @@ class TestLibvirtInspection(test.BaseTestCase): self.instance_name = 'instance-00000001' self.inspector = libvirt_inspector.LibvirtInspector() self.inspector.connection = mock.Mock() + libvirt_inspector.libvirt = mock.Mock() + libvirt_inspector.libvirt.VIR_DOMAIN_SHUTOFF = 5 self.domain = mock.Mock() self.addCleanup(mock.patch.stopall) @@ -144,7 +146,10 @@ class TestLibvirtInspection(test.BaseTestCase): return_value=dom_xml), mock.patch.object(self.domain, 'interfaceStats', - side_effect=interfaceStats)): + side_effect=interfaceStats), + mock.patch.object(self.domain, 'info', + return_value=(0L, 0L, 0L, + 2L, 999999L))): interfaces = list(self.inspector.inspect_vnics(self.instance_name)) self.assertEqual(len(interfaces), 3) @@ -186,6 +191,16 @@ class TestLibvirtInspection(test.BaseTestCase): self.assertEqual(info2.tx_bytes, 11L) self.assertEqual(info2.tx_packets, 12L) + def test_inspect_vnics_with_domain_shutoff(self): + connection = self.inspector.connection + with contextlib.nested(mock.patch.object(connection, 'lookupByName', + return_value=self.domain), + mock.patch.object(self.domain, 'info', + return_value=(5L, 0L, 0L, + 2L, 999999L))): + interfaces = list(self.inspector.inspect_vnics(self.instance_name)) + self.assertEqual(interfaces, []) + def test_inspect_disks(self): dom_xml = """ @@ -209,7 +224,10 @@ class TestLibvirtInspection(test.BaseTestCase): return_value=dom_xml), mock.patch.object(self.domain, 'blockStats', return_value=(1L, 2L, 3L, - 4L, -1))): + 4L, -1)), + mock.patch.object(self.domain, 'info', + return_value=(0L, 0L, 0L, + 2L, 999999L))): disks = list(self.inspector.inspect_disks(self.instance_name)) self.assertEqual(len(disks), 1) @@ -220,9 +238,22 @@ class TestLibvirtInspection(test.BaseTestCase): self.assertEqual(info0.write_requests, 3L) self.assertEqual(info0.write_bytes, 4L) + def test_inspect_disks_with_domain_shutoff(self): + connection = self.inspector.connection + with contextlib.nested(mock.patch.object(connection, 'lookupByName', + return_value=self.domain), + mock.patch.object(self.domain, 'info', + return_value=(5L, 0L, 0L, + 2L, 999999L))): + disks = list(self.inspector.inspect_disks(self.instance_name)) + self.assertEqual(disks, []) + class TestLibvirtInspectionWithError(test.BaseTestCase): + class fakeLibvirtError(Exception): + pass + def setUp(self): super(TestLibvirtInspectionWithError, self).setUp() self.inspector = libvirt_inspector.LibvirtInspector() @@ -230,11 +261,12 @@ class TestLibvirtInspectionWithError(test.BaseTestCase): 'ceilometer.compute.virt.libvirt.inspector.' 'LibvirtInspector._get_connection', self._dummy_get_connection)) + libvirt_inspector.libvirt = mock.Mock() + libvirt_inspector.libvirt.libvirtError = self.fakeLibvirtError def _dummy_get_connection(*args, **kwargs): raise Exception('dummy') def test_inspect_unknown_error(self): - self.assertRaises(virt_inspector.InspectorException, self.inspector.inspect_cpus, 'foo')