libvirt: Remove slowpath listing of instances
Commit 'e495d139' introduced a "fast path" method for listing all domains. This requires libvirt 0.9.13 [1] but, as the minimum version of libvirt at the time was 0.9.6 [2], the older "slow path" method was retained. Seeing as nova has long since moved on from 0.9.13, this "slow path" can be dropped and the tests simplified to handle the single path remaining. The (now unused) '_get_domain_by_id' function and its respective tests are obliterated in the process. This also fixes the 'listAllDomains' function of fakelibvirt.Connection, which appears not to have been called previously and was broken. [1] https://github.com/openstack/nova/blob/e495d139/nova/virt/libvirt/driver.py#L774 [2] https://github.com/openstack/nova/blob/e495d139/nova/virt/libvirt/driver.py#L289 Change-Id: I6a01529b4b98f5d29c90a7ba38da7e9337dae99b
This commit is contained in:
Stephen Finucane
parent
bce8e4b9e3
commit
49753c02cf
|
|
@ -1043,12 +1043,12 @@ class Connection(object):
|
|||
|
||||
def listAllDomains(self, flags):
|
||||
vms = []
|
||||
for vm in self._vms:
|
||||
for vm in self._vms.values():
|
||||
if flags & VIR_CONNECT_LIST_DOMAINS_ACTIVE:
|
||||
if vm.state != VIR_DOMAIN_SHUTOFF:
|
||||
if vm._state != VIR_DOMAIN_SHUTOFF:
|
||||
vms.append(vm)
|
||||
if flags & VIR_CONNECT_LIST_DOMAINS_INACTIVE:
|
||||
if vm.state == VIR_DOMAIN_SHUTOFF:
|
||||
if vm._state == VIR_DOMAIN_SHUTOFF:
|
||||
vms.append(vm)
|
||||
return vms
|
||||
|
||||
|
|
|
|||
|
|
@ -413,31 +413,6 @@ class HostTestCase(test.NoDBTestCase):
|
|||
self.assertTrue(self.host.has_version(lv_ver, None, hv_type))
|
||||
self.assertTrue(self.host.has_version(None, hv_ver, hv_type))
|
||||
|
||||
@mock.patch.object(fakelibvirt.virConnect, "lookupByID")
|
||||
def test_get_domain_by_id(self, fake_lookup):
|
||||
dom = fakelibvirt.virDomain(self.host.get_connection(),
|
||||
"<domain id='7'/>")
|
||||
|
||||
fake_lookup.return_value = dom
|
||||
|
||||
self.assertEqual(dom, self.host._get_domain_by_id(7))
|
||||
|
||||
fake_lookup.assert_called_once_with(7)
|
||||
|
||||
@mock.patch.object(fakelibvirt.virConnect, "lookupByID")
|
||||
def test_get_domain_by_id_raises(self, fake_lookup):
|
||||
fake_lookup.side_effect = fakelibvirt.make_libvirtError(
|
||||
fakelibvirt.libvirtError,
|
||||
'Domain not found: no domain with matching id 7',
|
||||
error_code=fakelibvirt.VIR_ERR_NO_DOMAIN,
|
||||
error_domain=fakelibvirt.VIR_FROM_QEMU)
|
||||
|
||||
self.assertRaises(exception.InstanceNotFound,
|
||||
self.host._get_domain_by_id,
|
||||
7)
|
||||
|
||||
fake_lookup.assert_called_once_with(7)
|
||||
|
||||
@mock.patch.object(fakelibvirt.virConnect, "lookupByName")
|
||||
def test_get_domain_by_name(self, fake_lookup):
|
||||
dom = fakelibvirt.virDomain(self.host.get_connection(),
|
||||
|
|
@ -490,14 +465,15 @@ class HostTestCase(test.NoDBTestCase):
|
|||
fake_get_domain.assert_called_once_with("instance-0000007c")
|
||||
|
||||
@mock.patch.object(fakelibvirt.Connection, "listAllDomains")
|
||||
def test_list_instance_domains_fast(self, mock_list_all):
|
||||
def test_list_instance_domains(self, mock_list_all):
|
||||
vm0 = FakeVirtDomain(id=0, name="Domain-0") # Xen dom-0
|
||||
vm1 = FakeVirtDomain(id=3, name="instance00000001")
|
||||
vm2 = FakeVirtDomain(id=17, name="instance00000002")
|
||||
vm3 = FakeVirtDomain(name="instance00000003")
|
||||
vm4 = FakeVirtDomain(name="instance00000004")
|
||||
|
||||
def fake_list_all(flags):
|
||||
vms = []
|
||||
vms = [vm0]
|
||||
if flags & fakelibvirt.VIR_CONNECT_LIST_DOMAINS_ACTIVE:
|
||||
vms.extend([vm1, vm2])
|
||||
if flags & fakelibvirt.VIR_CONNECT_LIST_DOMAINS_INACTIVE:
|
||||
|
|
@ -506,7 +482,7 @@ class HostTestCase(test.NoDBTestCase):
|
|||
|
||||
mock_list_all.side_effect = fake_list_all
|
||||
|
||||
doms = self.host._list_instance_domains_fast()
|
||||
doms = self.host.list_instance_domains()
|
||||
|
||||
mock_list_all.assert_called_once_with(
|
||||
fakelibvirt.VIR_CONNECT_LIST_DOMAINS_ACTIVE)
|
||||
|
|
@ -516,11 +492,12 @@ class HostTestCase(test.NoDBTestCase):
|
|||
self.assertEqual(doms[0].name(), vm1.name())
|
||||
self.assertEqual(doms[1].name(), vm2.name())
|
||||
|
||||
doms = self.host._list_instance_domains_fast(only_running=False)
|
||||
doms = self.host.list_instance_domains(only_running=False)
|
||||
|
||||
mock_list_all.assert_called_once_with(
|
||||
fakelibvirt.VIR_CONNECT_LIST_DOMAINS_ACTIVE |
|
||||
fakelibvirt.VIR_CONNECT_LIST_DOMAINS_INACTIVE)
|
||||
mock_list_all.reset_mock()
|
||||
|
||||
self.assertEqual(len(doms), 4)
|
||||
self.assertEqual(doms[0].name(), vm1.name())
|
||||
|
|
@ -528,148 +505,16 @@ class HostTestCase(test.NoDBTestCase):
|
|||
self.assertEqual(doms[2].name(), vm3.name())
|
||||
self.assertEqual(doms[3].name(), vm4.name())
|
||||
|
||||
@mock.patch.object(fakelibvirt.Connection, "numOfDomains")
|
||||
@mock.patch.object(fakelibvirt.Connection, "listDefinedDomains")
|
||||
@mock.patch.object(fakelibvirt.Connection, "listDomainsID")
|
||||
@mock.patch.object(host.Host, "_get_domain_by_name")
|
||||
@mock.patch.object(host.Host, "_get_domain_by_id")
|
||||
def test_list_instance_domains_slow(self,
|
||||
mock_get_id, mock_get_name,
|
||||
mock_list_ids, mock_list_names,
|
||||
mock_num_ids):
|
||||
vm1 = FakeVirtDomain(id=3, name="instance00000001")
|
||||
vm2 = FakeVirtDomain(id=17, name="instance00000002")
|
||||
vm3 = FakeVirtDomain(name="instance00000003")
|
||||
vm4 = FakeVirtDomain(name="instance00000004")
|
||||
vms = [vm1, vm2, vm3, vm4]
|
||||
|
||||
def fake_get_domain_by_id(id):
|
||||
for vm in vms:
|
||||
if vm.ID() == id:
|
||||
return vm
|
||||
raise exception.InstanceNotFound(instance_id=id)
|
||||
|
||||
def fake_get_domain_by_name(name):
|
||||
for vm in vms:
|
||||
if vm.name() == name:
|
||||
return vm
|
||||
raise exception.InstanceNotFound(instance_id=name)
|
||||
|
||||
def fake_list_ids():
|
||||
# Include one ID that no longer exists
|
||||
return [vm1.ID(), vm2.ID(), 666]
|
||||
|
||||
def fake_list_names():
|
||||
# Include one name that no longer exists and
|
||||
# one dup from running list to show race in
|
||||
# transition from inactive -> running
|
||||
return [vm1.name(), vm3.name(), vm4.name(), "fishfood"]
|
||||
|
||||
mock_get_id.side_effect = fake_get_domain_by_id
|
||||
mock_get_name.side_effect = fake_get_domain_by_name
|
||||
mock_list_ids.side_effect = fake_list_ids
|
||||
mock_list_names.side_effect = fake_list_names
|
||||
mock_num_ids.return_value = 2
|
||||
|
||||
doms = self.host._list_instance_domains_slow()
|
||||
|
||||
mock_list_ids.assert_called_once_with()
|
||||
mock_num_ids.assert_called_once_with()
|
||||
self.assertFalse(mock_list_names.called)
|
||||
mock_list_ids.reset_mock()
|
||||
mock_list_names.reset_mock()
|
||||
mock_num_ids.reset_mock()
|
||||
|
||||
self.assertEqual(len(doms), 2)
|
||||
self.assertEqual(doms[0].name(), vm1.name())
|
||||
self.assertEqual(doms[1].name(), vm2.name())
|
||||
|
||||
doms = self.host._list_instance_domains_slow(only_running=False)
|
||||
|
||||
mock_list_ids.assert_called_once_with()
|
||||
mock_num_ids.assert_called_once_with()
|
||||
mock_list_names.assert_called_once_with()
|
||||
|
||||
self.assertEqual(len(doms), 4)
|
||||
self.assertEqual(doms[0].name(), vm1.name())
|
||||
self.assertEqual(doms[1].name(), vm2.name())
|
||||
self.assertEqual(doms[2].name(), vm3.name())
|
||||
self.assertEqual(doms[3].name(), vm4.name())
|
||||
|
||||
@mock.patch.object(fakelibvirt.Connection, "listAllDomains")
|
||||
@mock.patch.object(fakelibvirt.Connection, "numOfDomains")
|
||||
@mock.patch.object(fakelibvirt.Connection, "listDomainsID")
|
||||
@mock.patch.object(host.Host, "_get_domain_by_id")
|
||||
def test_list_instance_domains_fallback(self,
|
||||
mock_get_id, mock_list_ids,
|
||||
mock_num_ids, mock_list_all):
|
||||
vm1 = FakeVirtDomain(id=3, name="instance00000001")
|
||||
vm2 = FakeVirtDomain(id=17, name="instance00000002")
|
||||
vms = [vm1, vm2]
|
||||
|
||||
def fake_get_domain_by_id(id):
|
||||
for vm in vms:
|
||||
if vm.ID() == id:
|
||||
return vm
|
||||
raise exception.InstanceNotFound(instance_id=id)
|
||||
|
||||
def fake_list_doms():
|
||||
return [vm1.ID(), vm2.ID()]
|
||||
|
||||
def fake_list_all(flags):
|
||||
ex = fakelibvirt.make_libvirtError(
|
||||
fakelibvirt.libvirtError,
|
||||
"API is not supported",
|
||||
error_code=fakelibvirt.VIR_ERR_NO_SUPPORT)
|
||||
raise ex
|
||||
|
||||
mock_get_id.side_effect = fake_get_domain_by_id
|
||||
mock_list_ids.side_effect = fake_list_doms
|
||||
mock_num_ids.return_value = 2
|
||||
mock_list_all.side_effect = fake_list_all
|
||||
|
||||
doms = self.host.list_instance_domains()
|
||||
doms = self.host.list_instance_domains(only_guests=False)
|
||||
|
||||
mock_list_all.assert_called_once_with(
|
||||
fakelibvirt.VIR_CONNECT_LIST_DOMAINS_ACTIVE)
|
||||
mock_list_ids.assert_called_once_with()
|
||||
mock_num_ids.assert_called_once_with()
|
||||
mock_list_all.reset_mock()
|
||||
|
||||
self.assertEqual(len(doms), 2)
|
||||
self.assertEqual(doms[0].ID(), vm1.ID())
|
||||
self.assertEqual(doms[1].ID(), vm2.ID())
|
||||
|
||||
@mock.patch.object(host.Host, "_list_instance_domains_fast")
|
||||
def test_list_instance_domains_filtering(self, mock_list):
|
||||
vm0 = FakeVirtDomain(id=0, name="Domain-0") # Xen dom-0
|
||||
vm1 = FakeVirtDomain(id=3, name="instance00000001")
|
||||
vm2 = FakeVirtDomain(id=17, name="instance00000002")
|
||||
vm3 = FakeVirtDomain(name="instance00000003")
|
||||
vm4 = FakeVirtDomain(name="instance00000004")
|
||||
|
||||
mock_list.return_value = [vm0, vm1, vm2]
|
||||
doms = self.host.list_instance_domains()
|
||||
self.assertEqual(len(doms), 2)
|
||||
self.assertEqual(doms[0].name(), vm1.name())
|
||||
self.assertEqual(doms[1].name(), vm2.name())
|
||||
mock_list.assert_called_with(True)
|
||||
|
||||
mock_list.return_value = [vm0, vm1, vm2, vm3, vm4]
|
||||
doms = self.host.list_instance_domains(only_running=False)
|
||||
self.assertEqual(len(doms), 4)
|
||||
self.assertEqual(doms[0].name(), vm1.name())
|
||||
self.assertEqual(doms[1].name(), vm2.name())
|
||||
self.assertEqual(doms[2].name(), vm3.name())
|
||||
self.assertEqual(doms[3].name(), vm4.name())
|
||||
mock_list.assert_called_with(False)
|
||||
|
||||
mock_list.return_value = [vm0, vm1, vm2]
|
||||
doms = self.host.list_instance_domains(only_guests=False)
|
||||
self.assertEqual(len(doms), 3)
|
||||
self.assertEqual(doms[0].name(), vm0.name())
|
||||
self.assertEqual(doms[1].name(), vm1.name())
|
||||
self.assertEqual(doms[2].name(), vm2.name())
|
||||
mock_list.assert_called_with(True)
|
||||
|
||||
@mock.patch.object(host.Host, "list_instance_domains")
|
||||
def test_list_guests(self, mock_list_domains):
|
||||
|
|
|
|||
|
|
@ -90,7 +90,6 @@ class Host(object):
|
|||
self._conn_event_handler = conn_event_handler
|
||||
self._conn_event_handler_queue = six.moves.queue.Queue()
|
||||
self._lifecycle_event_handler = lifecycle_event_handler
|
||||
self._skip_list_all_domains = False
|
||||
self._caps = None
|
||||
self._hostname = None
|
||||
|
||||
|
|
@ -545,28 +544,6 @@ class Host(object):
|
|||
return libvirt_guest.Guest(
|
||||
self.get_domain(instance))
|
||||
|
||||
def _get_domain_by_id(self, instance_id):
|
||||
"""Retrieve libvirt domain object given an instance id.
|
||||
|
||||
All libvirt error handling should be handled in this method and
|
||||
relevant nova exceptions should be raised in response.
|
||||
|
||||
"""
|
||||
try:
|
||||
conn = self.get_connection()
|
||||
return conn.lookupByID(instance_id)
|
||||
except libvirt.libvirtError as ex:
|
||||
error_code = ex.get_error_code()
|
||||
if error_code == libvirt.VIR_ERR_NO_DOMAIN:
|
||||
raise exception.InstanceNotFound(instance_id=instance_id)
|
||||
|
||||
msg = (_("Error from libvirt while looking up %(instance_id)s: "
|
||||
"[Error Code %(error_code)s] %(ex)s")
|
||||
% {'instance_id': instance_id,
|
||||
'error_code': error_code,
|
||||
'ex': ex})
|
||||
raise exception.NovaException(msg)
|
||||
|
||||
def _get_domain_by_name(self, instance_name):
|
||||
"""Retrieve libvirt domain object given an instance name.
|
||||
|
||||
|
|
@ -589,40 +566,6 @@ class Host(object):
|
|||
'ex': ex})
|
||||
raise exception.NovaException(msg)
|
||||
|
||||
def _list_instance_domains_fast(self, only_running=True):
|
||||
# The modern (>= 0.9.13) fast way - 1 single API call for all domains
|
||||
flags = libvirt.VIR_CONNECT_LIST_DOMAINS_ACTIVE
|
||||
if not only_running:
|
||||
flags = flags | libvirt.VIR_CONNECT_LIST_DOMAINS_INACTIVE
|
||||
return self.get_connection().listAllDomains(flags)
|
||||
|
||||
def _list_instance_domains_slow(self, only_running=True):
|
||||
# The legacy (< 0.9.13) slow way - O(n) API call for n domains
|
||||
uuids = []
|
||||
doms = []
|
||||
# Redundant numOfDomains check is for libvirt bz #836647
|
||||
if self.get_connection().numOfDomains() > 0:
|
||||
for id in self.get_connection().listDomainsID():
|
||||
try:
|
||||
dom = self._get_domain_by_id(id)
|
||||
doms.append(dom)
|
||||
uuids.append(dom.UUIDString())
|
||||
except exception.InstanceNotFound:
|
||||
continue
|
||||
|
||||
if only_running:
|
||||
return doms
|
||||
|
||||
for name in self.get_connection().listDefinedDomains():
|
||||
try:
|
||||
dom = self._get_domain_by_name(name)
|
||||
if dom.UUIDString() not in uuids:
|
||||
doms.append(dom)
|
||||
except exception.InstanceNotFound:
|
||||
continue
|
||||
|
||||
return doms
|
||||
|
||||
def list_guests(self, only_running=True, only_guests=True):
|
||||
"""Get a list of Guest objects for nova instances
|
||||
|
||||
|
|
@ -651,20 +594,10 @@ class Host(object):
|
|||
|
||||
:returns: list of libvirt.Domain objects
|
||||
"""
|
||||
|
||||
if not self._skip_list_all_domains:
|
||||
try:
|
||||
alldoms = self._list_instance_domains_fast(only_running)
|
||||
except (libvirt.libvirtError, AttributeError) as ex:
|
||||
LOG.info(_LI("Unable to use bulk domain list APIs, "
|
||||
"falling back to slow code path: %(ex)s"),
|
||||
{'ex': ex})
|
||||
self._skip_list_all_domains = True
|
||||
|
||||
if self._skip_list_all_domains:
|
||||
# Old libvirt, or a libvirt driver which doesn't
|
||||
# implement the new API
|
||||
alldoms = self._list_instance_domains_slow(only_running)
|
||||
flags = libvirt.VIR_CONNECT_LIST_DOMAINS_ACTIVE
|
||||
if not only_running:
|
||||
flags = flags | libvirt.VIR_CONNECT_LIST_DOMAINS_INACTIVE
|
||||
alldoms = self.get_connection().listAllDomains(flags)
|
||||
|
||||
doms = []
|
||||
for dom in alldoms:
|
||||
|
|
|
|||
Loading…
Reference in New Issue