From e521616afd14e6eedc0d3bfca0adf2179f4e05c2 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Fri, 8 Dec 2017 13:15:50 -0500 Subject: [PATCH] Don't launch guestfs in a thread pool if guestfs.debug is enabled When guestfs.debug is enabled, we're handling callback events from guestfs and logging them at debug level. When guestfs is launched to inspect capabilities, that is currently done in an eventlet thread pool. Because of the concurrent logging along with the eventlet thread, we can hit an issue where eventlet tries to switch threads and fails and then we hang the launch call to guestfs, which hangs creating an instance. This change simply avoids using a thread pool to launch guestfs if guestfs.debug is True. Change-Id: I0ffe93a031154b123c8beff96a695df5a280b935 Closes-Bug: #1737214 (cherry picked from commit 7c30da13842469e641d171b6008a57d5db8c5d5e) --- nova/tests/unit/virt/disk/vfs/test_guestfs.py | 17 ++++++++++++++++- nova/virt/disk/vfs/guestfs.py | 9 ++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/nova/tests/unit/virt/disk/vfs/test_guestfs.py b/nova/tests/unit/virt/disk/vfs/test_guestfs.py index 9cb879f70959..d0bd542ccfce 100644 --- a/nova/tests/unit/virt/disk/vfs/test_guestfs.py +++ b/nova/tests/unit/virt/disk/vfs/test_guestfs.py @@ -335,7 +335,8 @@ class VirtDiskVFSGuestFSTest(test.NoDBTestCase): m.launch.side_effect = Exception vfs = vfsimpl.VFSGuestFS(self.qcowfile) mock_access.return_value = False - with mock.patch('eventlet.tpool.Proxy', return_value=m): + self.flags(debug=False, group='guestfs') + with mock.patch('eventlet.tpool.Proxy', return_value=m) as tpool_mock: self.assertRaises(exception.LibguestfsCannotReadKernel, vfs.inspect_capabilities) m.add_drive.assert_called_once_with('/dev/null') @@ -343,3 +344,17 @@ class VirtDiskVFSGuestFSTest(test.NoDBTestCase): mock_access.assert_called_once_with('/boot/vmlinuz-kernel_name', mock.ANY) mock_uname.assert_called_once_with() + self.assertEqual(1, tpool_mock.call_count) + + def test_appliance_setup_inspect_capabilties_debug_mode(self): + """Asserts that we do not use an eventlet thread pool when guestfs + debug logging is enabled. + """ + # We can't actually mock guestfs.GuestFS because it's an optional + # native package import. All we really care about here is that + # eventlet isn't used. + self.flags(debug=True, group='guestfs') + vfs = vfsimpl.VFSGuestFS(self.qcowfile) + with mock.patch('eventlet.tpool.Proxy', + new_callable=mock.NonCallableMock): + vfs.inspect_capabilities() diff --git a/nova/virt/disk/vfs/guestfs.py b/nova/virt/disk/vfs/guestfs.py index 6e848bb72d82..84dddd875f37 100644 --- a/nova/virt/disk/vfs/guestfs.py +++ b/nova/virt/disk/vfs/guestfs.py @@ -75,7 +75,14 @@ class VFSGuestFS(vfs.VFS): def inspect_capabilities(self): """Determines whether guestfs is well configured.""" try: - g = tpool.Proxy(guestfs.GuestFS()) + # If guestfs debug is enabled, we can't launch in a thread because + # the debug logging callback can make eventlet try to switch + # threads and then the launch hangs, causing eternal sadness. + if CONF.guestfs.debug: + LOG.debug('Inspecting guestfs capabilities non-threaded.') + g = guestfs.GuestFS() + else: + g = tpool.Proxy(guestfs.GuestFS()) g.add_drive("/dev/null") # sic g.launch() except Exception as e: