Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Merge "libvirt: speed up _get_disk_over_committed_size_total method"

  • Loading branch information...
commit a62aae8d54ebda9206febce07680e82480b75f59 2 parents 0b3f458 + 4fc6f87
Jenkins authored openstack-gerrit committed
Showing with 107 additions and 52 deletions.
  1. +94 −44 nova/tests/virt/libvirt/test_driver.py
  2. +13 −8 nova/virt/libvirt/driver.py
View
138 nova/tests/virt/libvirt/test_driver.py
@@ -6317,58 +6317,108 @@ def fake_lookup(instance_name):
# instance disappears
conn._undefine_domain(instance)
- def test_disk_over_committed_size_total(self):
+ @mock.patch.object(libvirt_driver.LibvirtDriver,
+ "_list_instance_domains")
+ def test_disk_over_committed_size_total(self, mock_list):
# Ensure destroy calls managedSaveRemove for saved instance.
- conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+ class DiagFakeDomain(object):
+ def __init__(self, name):
+ self._name = name
+
+ def ID(self):
+ return 1
+
+ def name(self):
+ return self._name
+
+ def UUIDString(self):
+ return "19479fee-07a5-49bb-9138-d3738280d63c"
+
+ def XMLDesc(self, flags):
+ return "<domain/>"
+
+ mock_list.return_value = [
+ DiagFakeDomain("instance0000001"),
+ DiagFakeDomain("instance0000002")]
- def list_instances():
- return ['fake1', 'fake2']
- self.stubs.Set(conn, 'list_instances', list_instances)
-
- fake_disks = {'fake1': [{'type': 'qcow2', 'path': '/somepath/disk1',
- 'virt_disk_size': '10737418240',
- 'backing_file': '/somepath/disk1',
- 'disk_size': '83886080',
- 'over_committed_disk_size': '10653532160'}],
- 'fake2': [{'type': 'raw', 'path': '/somepath/disk2',
- 'virt_disk_size': '0',
- 'backing_file': '/somepath/disk2',
- 'disk_size': '10737418240',
- 'over_committed_disk_size': '0'}]}
-
- def get_info(instance_name, block_device_mapping=None):
+ drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+
+ fake_disks = {'instance0000001':
+ [{'type': 'qcow2', 'path': '/somepath/disk1',
+ 'virt_disk_size': '10737418240',
+ 'backing_file': '/somepath/disk1',
+ 'disk_size': '83886080',
+ 'over_committed_disk_size': '10653532160'}],
+ 'instance0000002':
+ [{'type': 'raw', 'path': '/somepath/disk2',
+ 'virt_disk_size': '0',
+ 'backing_file': '/somepath/disk2',
+ 'disk_size': '10737418240',
+ 'over_committed_disk_size': '0'}]}
+
+ def get_info(instance_name, xml, **kwargs):
return jsonutils.dumps(fake_disks.get(instance_name))
- self.stubs.Set(conn, 'get_instance_disk_info', get_info)
-
- result = conn._get_disk_over_committed_size_total()
- self.assertEqual(result, 10653532160)
-
- def test_disk_over_committed_size_total_permission_denied(self):
- driver = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
- driver.list_instances = mock.Mock(return_value=['fake1', 'fake2'])
-
- fake_disks = {'fake1': [{'type': 'qcow2', 'path': '/somepath/disk1',
- 'virt_disk_size': '10737418240',
- 'backing_file': '/somepath/disk1',
- 'disk_size': '83886080',
- 'over_committed_disk_size': '10653532160'}],
- 'fake2': [{'type': 'raw', 'path': '/somepath/disk2',
- 'virt_disk_size': '0',
- 'backing_file': '/somepath/disk2',
- 'disk_size': '10737418240',
- 'over_committed_disk_size': '21474836480'}]}
-
- def side_effect(arg):
- if arg == 'fake1':
+
+ with mock.patch.object(drvr,
+ "_get_instance_disk_info") as mock_info:
+ mock_info.side_effect = get_info
+
+ result = drvr._get_disk_over_committed_size_total()
+ self.assertEqual(result, 10653532160)
+ mock_list.assert_called_with()
+ mock_info.assert_called()
+
+ @mock.patch.object(libvirt_driver.LibvirtDriver,
+ "_list_instance_domains")
+ def test_disk_over_committed_size_total_eperm(self, mock_list):
+ # Ensure destroy calls managedSaveRemove for saved instance.
+ class DiagFakeDomain(object):
+ def __init__(self, name):
+ self._name = name
+
+ def ID(self):
+ return 1
+
+ def name(self):
+ return self._name
+
+ def UUIDString(self):
+ return "19479fee-07a5-49bb-9138-d3738280d63c"
+
+ def XMLDesc(self, flags):
+ return "<domain/>"
+
+ mock_list.return_value = [
+ DiagFakeDomain("instance0000001"),
+ DiagFakeDomain("instance0000002")]
+
+ drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+
+ fake_disks = {'instance0000001':
+ [{'type': 'qcow2', 'path': '/somepath/disk1',
+ 'virt_disk_size': '10737418240',
+ 'backing_file': '/somepath/disk1',
+ 'disk_size': '83886080',
+ 'over_committed_disk_size': '10653532160'}],
+ 'instance0000002':
+ [{'type': 'raw', 'path': '/somepath/disk2',
+ 'virt_disk_size': '0',
+ 'backing_file': '/somepath/disk2',
+ 'disk_size': '10737418240',
+ 'over_committed_disk_size': '21474836480'}]}
+
+ def side_effect(name, dom):
+ if name == 'instance0000001':
raise OSError(errno.EACCES, 'Permission denied')
- if arg == 'fake2':
- return jsonutils.dumps(fake_disks.get(arg))
+ if name == 'instance0000002':
+ return jsonutils.dumps(fake_disks.get(name))
get_disk_info = mock.Mock()
get_disk_info.side_effect = side_effect
- driver.get_instance_disk_info = get_disk_info
+ drvr._get_instance_disk_info = get_disk_info
- result = driver._get_disk_over_committed_size_total()
+ result = drvr._get_disk_over_committed_size_total()
self.assertEqual(21474836480, result)
+ mock_list.assert_called_with()
def test_cpu_info(self):
conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
View
21 nova/virt/libvirt/driver.py
@@ -4975,34 +4975,39 @@ def get_instance_disk_info(self, instance_name,
def _get_disk_over_committed_size_total(self):
"""Return total over committed disk size for all instances."""
# Disk size that all instance uses : virtual_size - disk_size
- instances_name = self.list_instances()
disk_over_committed_size = 0
- for i_name in instances_name:
+ for dom in self._list_instance_domains():
try:
+ xml = dom.XMLDesc(0)
disk_infos = jsonutils.loads(
- self.get_instance_disk_info(i_name))
+ self._get_instance_disk_info(dom.name(), xml))
for info in disk_infos:
disk_over_committed_size += int(
info['over_committed_disk_size'])
+ except libvirt.libvirtError as ex:
+ error_code = ex.get_error_code()
+ LOG.warn(_LW(
+ 'Error from libvirt while getting description of '
+ '%(instance_name)s: [Error Code %(error_code)s] %(ex)s'
+ ) % {'instance_name': dom.name(),
+ 'error_code': error_code,
+ 'ex': ex})
except OSError as e:
if e.errno == errno.ENOENT:
LOG.warn(_LW('Periodic task is updating the host stat, '
'it is trying to get disk %(i_name)s, '
'but disk file was removed by concurrent '
'operations such as resize.'),
- {'i_name': i_name})
+ {'i_name': dom.name()})
if e.errno == errno.EACCES:
LOG.warn(_LW('Periodic task is updating the host stat, '
'it is trying to get disk %(i_name)s, '
'but access is denied. It is most likely '
'due to a VM that exists on the compute '
'node but is not managed by Nova.'),
- {'i_name': i_name})
+ {'i_name': dom.name()})
else:
raise
- except exception.InstanceNotFound:
- # Instance was deleted during the check so ignore it
- pass
# NOTE(gtt116): give other tasks a chance.
greenthread.sleep(0)
return disk_over_committed_size
Please sign in to comment.
Something went wrong with that request. Please try again.