openstack
/
nova
Code Issues Proposed changes

libvirt: Fix getting a wrong guest object 

If trying to create a new VM with the same instance name on the same
compute host, old existing VM become destroyed. guest object is get by
instance name so returned object is existing active instance and it is
destroyed. However this destroying is not intended situation. This
commit makes get the correct guest object by using UUID.

Co-Authored-By: Maciej Kucia <m.kucia@partner.samsung.com>
Change-Id: Ic6f81dc1f8b3610e181914f6d977652cb6d3f6d0
Closes-Bug: #1712460
Signed-off-by: Wonil Choi <wonil22.choi@samsung.com>
Signed-off-by: Maciej Kucia <m.kucia@partner.samsung.com>
(cherry picked from commit ac4705516a)
This commit is contained in:
Wonil Choi 2017-08-23 15:33:02 +09:00 committed by Sean Dague
parent 8b162ba21f
commit 98f0d81621
5 changed files with 83 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
nova/tests/unit/virt/libvirt/fakelibvirt.py
View File

@ -1054,12 +1054,13 @@ class Connection(object):
self.host_info.cpu_cores,
self.host_info.cpu_threads]
def lookupByName(self, name):
if name in self._vms:
return self._vms[name]
def lookupByUUIDString(self, uuid):
for vm in self._vms.values():
if vm.UUIDString() == uuid:
return vm
raise make_libvirtError(
libvirtError,
'Domain not found: no domain with matching name "%s"' % name,
'Domain not found: no domain with matching uuid "%s"' % uuid,
error_code=VIR_ERR_NO_DOMAIN,
error_domain=VIR_FROM_QEMU)

62
nova/tests/unit/virt/libvirt/test_driver.py
View File

@ -6270,7 +6270,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
@mock.patch.object(host.Host, "has_min_version", return_value=True)
def test_quiesce(self, mock_has_min_version):
self.create_fake_libvirt_mock(lookupByName=self.fake_lookup)
self.create_fake_libvirt_mock(lookupByUUIDString=self.fake_lookup)
with mock.patch.object(FakeVirtDomain, "fsFreeze") as mock_fsfreeze:
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())
instance = objects.Instance(**self.test_instance)
@ -6282,7 +6282,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
@mock.patch.object(host.Host, "has_min_version", return_value=True)
def test_unquiesce(self, mock_has_min_version):
self.create_fake_libvirt_mock(getLibVersion=lambda: 1002005,
lookupByName=self.fake_lookup)
lookupByUUIDString=self.fake_lookup)
with mock.patch.object(FakeVirtDomain, "fsThaw") as mock_fsthaw:
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())
instance = objects.Instance(**self.test_instance)
@ -6377,7 +6377,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
def test_attach_invalid_volume_type(self):
self.create_fake_libvirt_mock()
libvirt_driver.LibvirtDriver._conn.lookupByName = self.fake_lookup
libvirt_driver.LibvirtDriver._conn.lookupByUUIDString \
= self.fake_lookup
instance = objects.Instance(**self.test_instance)
self.mox.ReplayAll()
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
@ -6390,7 +6391,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
def test_attach_blockio_invalid_hypervisor(self):
self.flags(virt_type='lxc', group='libvirt')
self.create_fake_libvirt_mock()
libvirt_driver.LibvirtDriver._conn.lookupByName = self.fake_lookup
libvirt_driver.LibvirtDriver._conn.lookupByUUIDString \
= self.fake_lookup
instance = objects.Instance(**self.test_instance)
self.mox.ReplayAll()
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
@ -10278,10 +10280,10 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.mox.StubOutWithMock(vdmock, "XMLDesc")
vdmock.XMLDesc(0).AndReturn(dummyxml)
def fake_lookup(instance_name):
if instance_name == instance.name:
def fake_lookup(_uuid):
if _uuid == instance.uuid:
return vdmock
self.create_fake_libvirt_mock(lookupByName=fake_lookup)
self.create_fake_libvirt_mock(lookupByUUIDString=fake_lookup)
fake_libvirt_utils.disk_sizes['/test/disk'] = 10 * units.Gi
fake_libvirt_utils.disk_sizes['/test/disk.local'] = 20 * units.Gi
@ -10388,10 +10390,10 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.mox.StubOutWithMock(vdmock, "XMLDesc")
vdmock.XMLDesc(0).AndReturn(dummyxml)
def fake_lookup(instance_name):
if instance_name == instance.name:
def fake_lookup(_uuid):
if _uuid == instance.uuid:
return vdmock
self.create_fake_libvirt_mock(lookupByName=fake_lookup)
self.create_fake_libvirt_mock(lookupByUUIDString=fake_lookup)
fake_libvirt_utils.disk_sizes['/test/disk'] = 10 * units.Gi
fake_libvirt_utils.disk_sizes['/test/disk.local'] = 20 * units.Gi
@ -10456,10 +10458,10 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.mox.StubOutWithMock(vdmock, "XMLDesc")
vdmock.XMLDesc(0).AndReturn(dummyxml)
def fake_lookup(instance_name):
if instance_name == instance.name:
def fake_lookup(_uuid):
if _uuid == instance.uuid:
return vdmock
self.create_fake_libvirt_mock(lookupByName=fake_lookup)
self.create_fake_libvirt_mock(lookupByUUIDString=fake_lookup)
fake_libvirt_utils.disk_sizes['/test/disk'] = 10 * units.Gi
@ -11292,7 +11294,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
return FakeVirtDomain(fake_dom_xml)
self.create_fake_libvirt_mock()
libvirt_driver.LibvirtDriver._conn.lookupByName = fake_lookup
libvirt_driver.LibvirtDriver._conn.lookupByUUIDString = fake_lookup
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
@ -11334,7 +11336,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
return FakeVirtDomain(fake_dom_xml)
self.create_fake_libvirt_mock()
libvirt_driver.LibvirtDriver._conn.lookupByName = fake_lookup
libvirt_driver.LibvirtDriver._conn.lookupByUUIDString = fake_lookup
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
@ -11380,7 +11382,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
return 'pty'
self.create_fake_libvirt_mock()
libvirt_driver.LibvirtDriver._conn.lookupByName = fake_lookup
libvirt_driver.LibvirtDriver._conn.lookupByUUIDString = fake_lookup
libvirt_driver.LibvirtDriver._flush_libvirt_console = _fake_flush
libvirt_driver.LibvirtDriver._append_to_file = _fake_append_to_file
@ -11414,7 +11416,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
return FakeVirtDomain(fake_dom_xml)
self.create_fake_libvirt_mock()
libvirt_driver.LibvirtDriver._conn.lookupByName = fake_lookup
libvirt_driver.LibvirtDriver._conn.lookupByUUIDString = fake_lookup
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
self.assertRaises(exception.ConsoleNotAvailable,
drvr.get_console_output, self.context, instance)
@ -14167,10 +14169,10 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.mox.StubOutWithMock(vdmock, "XMLDesc")
vdmock.XMLDesc(flags=0).AndReturn(dummyxml)
def fake_lookup(instance_name):
if instance_name == instance['name']:
def fake_lookup(_uuid):
if _uuid == instance['uuid']:
return vdmock
self.create_fake_libvirt_mock(lookupByName=fake_lookup)
self.create_fake_libvirt_mock(lookupByUUIDString=fake_lookup)
self.mox.ReplayAll()
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
@ -14186,10 +14188,10 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.mox.StubOutWithMock(vdmock, "XMLDesc")
vdmock.XMLDesc(flags=0).AndReturn(dummyxml)
def fake_lookup(instance_name):
if instance_name == instance['name']:
def fake_lookup(_uuid):
if _uuid == instance['uuid']:
return vdmock
self.create_fake_libvirt_mock(lookupByName=fake_lookup)
self.create_fake_libvirt_mock(lookupByUUIDString=fake_lookup)
self.mox.ReplayAll()
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
@ -14207,10 +14209,10 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.mox.StubOutWithMock(vdmock, "XMLDesc")
vdmock.XMLDesc(flags=0).AndReturn(dummyxml)
def fake_lookup(instance_name):
if instance_name == instance['name']:
def fake_lookup(_uuid):
if _uuid == instance['uuid']:
return vdmock
self.create_fake_libvirt_mock(lookupByName=fake_lookup)
self.create_fake_libvirt_mock(lookupByUUIDString=fake_lookup)
self.mox.ReplayAll()
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
@ -14226,10 +14228,10 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.mox.StubOutWithMock(vdmock, "XMLDesc")
vdmock.XMLDesc(flags=0).AndReturn(dummyxml)
def fake_lookup(instance_name):
if instance_name == instance['name']:
def fake_lookup(_uuid):
if _uuid == instance['uuid']:
return vdmock
self.create_fake_libvirt_mock(lookupByName=fake_lookup)
self.create_fake_libvirt_mock(lookupByUUIDString=fake_lookup)
self.mox.ReplayAll()
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
@ -14855,7 +14857,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
def test_cleanup_wants_vif_errors_ignored(self, undefine, unplug):
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())
fake_inst = {'name': 'foo'}
with mock.patch.object(drvr._conn, 'lookupByName') as lookup:
with mock.patch.object(drvr._conn, 'lookupByUUIDString') as lookup:
lookup.return_value = fake_inst
# NOTE(danms): Make unplug cause us to bail early, since
# we only care about how it was called

61
nova/tests/unit/virt/libvirt/test_fakelibvirt.py
View File

@ -13,6 +13,7 @@
# under the License.
from lxml import etree
from oslo_utils import uuidutils
import six
from nova.objects import fields as obj_fields
@ -127,29 +128,32 @@ class FakeLibvirtTests(test.NoDBTestCase):
raise self.failureException("Invalid XML didn't raise libvirtError")
def test_defineXML_defines_domain(self):
uuid = uuidutils.generate_uuid()
conn = self.get_openAuth_curry_func()('qemu:///system')
conn.defineXML(get_vm_xml())
dom = conn.lookupByName('testname')
conn.defineXML(get_vm_xml(uuid=uuid))
dom = conn.lookupByUUIDString(uuid)
self.assertEqual('testname', dom.name())
self.assertEqual(0, dom.isActive())
dom.undefine()
self.assertRaises(libvirt.libvirtError,
conn.lookupByName,
'testname')
conn.lookupByUUIDString,
uuid)
def test_blockStats(self):
uuid = uuidutils.generate_uuid()
conn = self.get_openAuth_curry_func()('qemu:///system')
conn.createXML(get_vm_xml(), 0)
dom = conn.lookupByName('testname')
conn.createXML(get_vm_xml(uuid=uuid), 0)
dom = conn.lookupByUUIDString(uuid)
blockstats = dom.blockStats('vda')
self.assertEqual(len(blockstats), 5)
for x in blockstats:
self.assertIn(type(x), six.integer_types)
def test_attach_detach(self):
uuid = uuidutils.generate_uuid()
conn = self.get_openAuth_curry_func()('qemu:///system')
conn.createXML(get_vm_xml(), 0)
dom = conn.lookupByName('testname')
conn.createXML(get_vm_xml(uuid=uuid), 0)
dom = conn.lookupByUUIDString(uuid)
xml = '''<disk type='block'>
<driver name='qemu' type='raw'/>
<source dev='/dev/nbd0'/>
@ -159,9 +163,10 @@ class FakeLibvirtTests(test.NoDBTestCase):
self.assertTrue(dom.detachDevice(xml))
def test_info(self):
uuid = uuidutils.generate_uuid()
conn = self.get_openAuth_curry_func()('qemu:///system')
conn.createXML(get_vm_xml(), 0)
dom = conn.lookupByName('testname')
conn.createXML(get_vm_xml(uuid=uuid), 0)
dom = conn.lookupByUUIDString(uuid)
info = dom.info()
self.assertEqual(info[0], libvirt.VIR_DOMAIN_RUNNING)
self.assertEqual(info[1], 128000)
@ -170,40 +175,43 @@ class FakeLibvirtTests(test.NoDBTestCase):
self.assertIn(type(info[4]), six.integer_types)
def test_createXML_runs_domain(self):
uuid = uuidutils.generate_uuid()
conn = self.get_openAuth_curry_func()('qemu:///system')
conn.createXML(get_vm_xml(), 0)
dom = conn.lookupByName('testname')
conn.createXML(get_vm_xml(uuid=uuid), 0)
dom = conn.lookupByUUIDString(uuid)
self.assertEqual('testname', dom.name())
self.assertEqual(1, dom.isActive())
dom.destroy()
try:
conn.lookupByName('testname')
conn.lookupByUUIDString(uuid)
except libvirt.libvirtError as e:
self.assertEqual(e.get_error_code(), libvirt.VIR_ERR_NO_DOMAIN)
self.assertEqual(e.get_error_domain(), libvirt.VIR_FROM_QEMU)
return
self.fail("lookupByName succeeded for destroyed non-defined VM")
self.fail("lookupByUUIDString succeeded for destroyed non-defined VM")
def test_defineXML_remembers_uuid(self):
conn = self.get_openAuth_curry_func()('qemu:///system')
uuid = 'b21f957d-a72f-4b93-b5a5-45b1161abb02'
conn.defineXML(get_vm_xml(uuid=uuid))
dom = conn.lookupByName('testname')
dom = conn.lookupByUUIDString(uuid)
self.assertEqual(dom.UUIDString(), uuid)
def test_createWithFlags(self):
uuid = uuidutils.generate_uuid()
conn = self.get_openAuth_curry_func()('qemu:///system')
conn.defineXML(get_vm_xml())
dom = conn.lookupByName('testname')
conn.defineXML(get_vm_xml(uuid=uuid))
dom = conn.lookupByUUIDString(uuid)
self.assertFalse(dom.isActive(), 'Defined domain was running.')
dom.createWithFlags(0)
self.assertTrue(dom.isActive(),
'Domain wasn\'t running after createWithFlags')
def test_managedSave(self):
uuid = uuidutils.generate_uuid()
conn = self.get_openAuth_curry_func()('qemu:///system')
conn.defineXML(get_vm_xml())
dom = conn.lookupByName('testname')
conn.defineXML(get_vm_xml(uuid=uuid))
dom = conn.lookupByUUIDString(uuid)
self.assertFalse(dom.isActive(), 'Defined domain was running.')
dom.createWithFlags(0)
self.assertEqual(dom.hasManagedSaveImage(0), 0)
@ -213,18 +221,20 @@ class FakeLibvirtTests(test.NoDBTestCase):
self.assertEqual(dom.hasManagedSaveImage(0), 0)
def test_define_and_retrieve(self):
uuid = uuidutils.generate_uuid()
conn = self.get_openAuth_curry_func()('qemu:///system')
self.assertEqual(conn.listAllDomains(), [])
conn.defineXML(get_vm_xml())
dom = conn.lookupByName('testname')
conn.defineXML(get_vm_xml(uuid=uuid))
dom = conn.lookupByUUIDString(uuid)
xml = dom.XMLDesc(0)
etree.fromstring(xml)
def _test_accepts_source_type(self, source_type):
uuid = uuidutils.generate_uuid()
conn = self.get_openAuth_curry_func()('qemu:///system')
self.assertEqual(conn.listAllDomains(), [])
conn.defineXML(get_vm_xml(source_type=source_type))
dom = conn.lookupByName('testname')
conn.defineXML(get_vm_xml(source_type=source_type, uuid=uuid))
dom = conn.lookupByUUIDString(uuid)
xml = dom.XMLDesc(0)
tree = etree.fromstring(xml)
elem = tree.find('./devices/disk/source')
@ -243,10 +253,11 @@ class FakeLibvirtTests(test.NoDBTestCase):
self._test_network_type_sticks('network')
def _test_network_type_sticks(self, network_type):
uuid = uuidutils.generate_uuid()
conn = self.get_openAuth_curry_func()('qemu:///system')
self.assertEqual(conn.listAllDomains(), [])
conn.defineXML(get_vm_xml(interface_type=network_type))
dom = conn.lookupByName('testname')
conn.defineXML(get_vm_xml(interface_type=network_type, uuid=uuid))
dom = conn.lookupByUUIDString(uuid)
xml = dom.XMLDesc(0)
tree = etree.fromstring(xml)
elem = tree.find('./devices/interface')

16
nova/tests/unit/virt/libvirt/test_host.py
View File

@ -421,17 +421,18 @@ 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, "lookupByName")
@mock.patch.object(fakelibvirt.virConnect, "lookupByUUIDString")
def test_get_domain(self, fake_lookup):
uuid = uuidutils.generate_uuid()
dom = fakelibvirt.virDomain(self.host.get_connection(),
"<domain id='7'/>")
instance = objects.Instance(id="124")
instance = objects.Instance(id="124", uuid=uuid)
fake_lookup.return_value = dom
self.assertEqual(dom, self.host.get_domain(instance))
fake_lookup.assert_called_once_with("instance-0000007c")
fake_lookup.assert_called_once_with(uuid)
@mock.patch.object(fakelibvirt.virConnect, "lookupByName")
@mock.patch.object(fakelibvirt.virConnect, "lookupByUUIDString")
def test_get_domain_raises(self, fake_lookup):
instance = objects.Instance(uuid=uuids.instance,
vm_state=vm_states.ACTIVE)
@ -446,19 +447,20 @@ class HostTestCase(test.NoDBTestCase):
fake_lookup.assert_called_once_with(uuids.instance)
@mock.patch.object(fakelibvirt.virConnect, "lookupByName")
@mock.patch.object(fakelibvirt.virConnect, "lookupByUUIDString")
def test_get_guest(self, fake_lookup):
uuid = uuidutils.generate_uuid()
dom = fakelibvirt.virDomain(self.host.get_connection(),
"<domain id='7'/>")
fake_lookup.return_value = dom
instance = objects.Instance(id="124")
instance = objects.Instance(id="124", uuid=uuid)
guest = self.host.get_guest(instance)
self.assertEqual(dom, guest._domain)
self.assertIsInstance(guest, libvirt_guest.Guest)
fake_lookup.assert_called_once_with("instance-0000007c")
fake_lookup.assert_called_once_with(uuid)
@mock.patch.object(fakelibvirt.Connection, "listAllDomains")
def test_list_instance_domains(self, mock_list_all):

2
nova/virt/libvirt/host.py
View File

@ -548,7 +548,7 @@ class Host(object):
"""
try:
conn = self.get_connection()
return conn.lookupByName(instance.name)
return conn.lookupByUUIDString(instance.uuid)
except libvirt.libvirtError as ex:
error_code = ex.get_error_code()
if error_code == libvirt.VIR_ERR_NO_DOMAIN: