From 95da4e87881424524c8ddda6a546764d43eb755e Mon Sep 17 00:00:00 2001 From: Sahid Orentino Ferdjaoui Date: Mon, 4 Apr 2016 10:24:23 -0400 Subject: [PATCH] libvirt: release serial console ports when destroying guests The part of code responsible to release serial ports does not cover all the cases. This commit moves that part inside the method _destroy which is responsible for destroying the guest from a libvirt perspective which is when we want to release ports. Change-Id: I24f2a1fa3b7ef3f2a3196f3689e5f2ba7d49bd87 Closes-Bug: #1489853 --- nova/tests/unit/virt/libvirt/test_driver.py | 92 ++++++--------------- nova/virt/libvirt/driver.py | 17 ++-- 2 files changed, 32 insertions(+), 77 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 07c54308db58..ef5abbf11ca8 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -3496,6 +3496,30 @@ class LibvirtConnTestCase(test.NoDBTestCase): drvr._get_guest_config, instance_ref, [], image_meta, disk_info) + @mock.patch('nova.console.serial.release_port') + @mock.patch.object(libvirt_driver.LibvirtDriver, 'get_info') + @mock.patch.object(host.Host, 'get_guest') + @mock.patch.object(libvirt_driver.LibvirtDriver, + '_get_serial_ports_from_guest') + def test_serial_console_release_port( + self, mock_get_serial_ports_from_guest, mock_get_guest, + mock_get_info, mock_release_port): + self.flags(enabled="True", group='serial_console') + + guest = libvirt_guest.Guest(FakeVirtDomain()) + guest.power_off = mock.Mock() + mock_get_info.return_value = hardware.InstanceInfo( + state=power_state.SHUTDOWN) + mock_get_guest.return_value = guest + mock_get_serial_ports_from_guest.return_value = iter([ + ('127.0.0.1', 10000), ('127.0.0.1', 10001)]) + + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + drvr._destroy(objects.Instance(**self.test_instance)) + mock_release_port.assert_has_calls( + [mock.call(host='127.0.0.1', port=10000), + mock.call(host='127.0.0.1', port=10001)]) + @mock.patch('os.path.getsize', return_value=0) # size doesn't matter @mock.patch('nova.virt.libvirt.storage.lvm.get_volume_size', return_value='fake-size') @@ -13149,74 +13173,6 @@ class LibvirtConnTestCase(test.NoDBTestCase): drvr.cleanup, 'ctxt', fake_inst, 'netinfo') unplug.assert_called_once_with(fake_inst, 'netinfo', True) - @mock.patch.object(driver, 'block_device_info_get_mapping') - @mock.patch.object(host.Host, "get_guest") - @mock.patch.object(libvirt_driver.LibvirtDriver, - '_get_serial_ports_from_guest') - @mock.patch.object(libvirt_driver.LibvirtDriver, '_undefine_domain') - def test_cleanup_serial_console_enabled( - self, undefine, get_ports, get_guest, - block_device_info_get_mapping): - self.flags(enabled="True", group='serial_console') - instance = 'i1' - network_info = {} - bdm_info = {} - firewall_driver = mock.MagicMock() - - guest = mock.Mock(spec=libvirt_guest.Guest) - get_guest.return_value = guest - get_ports.return_value = iter([('127.0.0.1', 10000)]) - block_device_info_get_mapping.return_value = () - - # We want to ensure undefine_domain is called after - # lookup_domain. - def undefine_domain(instance): - get_ports.side_effect = Exception("domain undefined") - undefine.side_effect = undefine_domain - - drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI()) - drvr.firewall_driver = firewall_driver - drvr.cleanup( - 'ctx', instance, network_info, - block_device_info=bdm_info, - destroy_disks=False, destroy_vifs=False) - - get_ports.assert_called_once_with(guest) - undefine.assert_called_once_with(instance) - firewall_driver.unfilter_instance.assert_called_once_with( - instance, network_info=network_info) - block_device_info_get_mapping.assert_called_once_with(bdm_info) - - @mock.patch.object(driver, 'block_device_info_get_mapping') - @mock.patch.object(host.Host, "get_guest") - @mock.patch.object(libvirt_driver.LibvirtDriver, '_undefine_domain') - def test_cleanup_serial_console_domain_gone( - self, undefine, get_guest, block_device_info_get_mapping): - self.flags(enabled="True", group='serial_console') - instance = {'name': 'i1'} - network_info = {} - bdm_info = {} - firewall_driver = mock.MagicMock() - - block_device_info_get_mapping.return_value = () - - # Ensure get_guest raises same exception that would have occurred - # if domain was gone. - get_guest.side_effect = exception.InstanceNotFound("domain undefined") - - drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI()) - drvr.firewall_driver = firewall_driver - drvr.cleanup( - 'ctx', instance, network_info, - block_device_info=bdm_info, - destroy_disks=False, destroy_vifs=False) - - get_guest.assert_called_once_with(instance) - undefine.assert_called_once_with(instance) - firewall_driver.unfilter_instance.assert_called_once_with( - instance, network_info=network_info) - block_device_info_get_mapping.assert_called_once_with(bdm_info) - @mock.patch.object(libvirt_driver.LibvirtDriver, 'unfilter_instance') @mock.patch.object(libvirt_driver.LibvirtDriver, 'delete_instance_files', return_value=True) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 5f644057ac90..f5c45de5509d 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -907,6 +907,14 @@ class LibvirtDriver(driver.ComputeDriver): def _destroy(self, instance, attempt=1): try: guest = self._host.get_guest(instance) + if CONF.serial_console.enabled: + # This method is called for several events: destroy, + # rebuild, hard-reboot, power-off - For all of these + # events we want to release the serial ports acquired + # for the guest before destroying it. + serials = self._get_serial_ports_from_guest(guest) + for hostname, port in serials: + serial_console.release_port(host=hostname, port=port) except exception.InstanceNotFound: guest = None @@ -1146,15 +1154,6 @@ class LibvirtDriver(driver.ComputeDriver): instance.cleaned = True instance.save() - if CONF.serial_console.enabled: - try: - guest = self._host.get_guest(instance) - serials = self._get_serial_ports_from_guest(guest) - for hostname, port in serials: - serial_console.release_port(host=hostname, port=port) - except exception.InstanceNotFound: - pass - self._undefine_domain(instance) def _detach_encrypted_volumes(self, instance, block_device_info):