Revert "Add Serial-over-LAN (SOL) support"
This reverts commit 9f7bfb9f6b.
It seems that the SOL feature introduced by patch [1] led to
the VirtualBMC daemons leaking two file descriptors (ends of
UNIX pipe, perhaps) per each IPMI query it processes. Given the
default ulimit of 1024 open files per process, in circa 500 IPMI
queries vbmc renders itself not quite functional.
The quick research of the problem reveals that the leak comes
to life once the virEventRegisterDefaultImpl() call is made
which prepares libvirt for asynchronous operations over a dedicated
connection to libvirtd. Even though once SOL session is gone and
libvirt is cleaned up (well, dedicated connection is closed),
the libvirt connection, which is open every time when IPMI query
comes in, leaks 2 FDs per query.
This problem has been reported about some other libvirt application
in the past [2] so the problem might go away if we turn the IPMI
part of vbmc to the asynchronous operation mode as well.
Another observation is that libvirt API may not [3] offer a full
clean up upon exiting from the asynchronous operation mode.
To summarize: the real cause of this behaviour is not yet properly
understood.
1. https://review.openstack.org/#/c/482853/
2. https://www.redhat.com/archives/libvir-list/2013-September/msg00019.html
3. https://github.com/libvirt/libvirt/blob/master/src/util/virevent.c#L224
Change-Id: Id599f62e0eaf8c96e4552d2cd263f93155f8afa6
Story: 2001976
Task: 19585
This commit is contained in:
Ilya Etingof
parent
2eea62aa6e
commit
30e168e792
|
|
@ -31,7 +31,7 @@ Useful examples
|
|||
* Adding a new virtual BMC to control a domain called ``node-1`` that
|
||||
will listen on the port ``6230``::
|
||||
|
||||
$ vbmc add node-1 --port 6230
|
||||
$ vbmc add node-0 --port 6230
|
||||
|
||||
|
||||
.. note::
|
||||
|
|
@ -55,8 +55,8 @@ Useful examples
|
|||
+-------------+---------+---------+------+
|
||||
| Domain name | Status | Address | Port |
|
||||
+-------------+---------+---------+------+
|
||||
| node-0 | running | :: | 623 |
|
||||
| node-1 | running | :: | 6230 |
|
||||
| node-0 | running | :: | 6230 |
|
||||
| node-1 | running | :: | 6231 |
|
||||
+-------------+---------+---------+------+
|
||||
|
||||
|
||||
|
|
@ -72,7 +72,7 @@ Useful examples
|
|||
| libvirt_sasl_username | None |
|
||||
| libvirt_uri | qemu:///system |
|
||||
| password | *** |
|
||||
| port | 623 |
|
||||
| port | 6230 |
|
||||
| status | running |
|
||||
| username | admin |
|
||||
+-----------------------+----------------+
|
||||
|
|
@ -100,12 +100,3 @@ virtual BMC to control the libvirt domain. For example:
|
|||
* To get the current boot device::
|
||||
|
||||
$ ipmitool -I lanplus -U admin -P password -H 127.0.0.1 -p 6230 chassis bootparam get 5
|
||||
|
||||
* To connect the serial console (type "~." to quit)::
|
||||
|
||||
$ ipmitool -I lanplus -U admin -P password -H 127.0.0.1 -p 6230 sol activate
|
||||
|
||||
.. warning::
|
||||
|
||||
Old versions of pyghmi support only port 623 and 28418 for serial
|
||||
console. Use pyghmi-1.0.32 or later.
|
||||
|
|
|
|||
|
|
@ -15,9 +15,7 @@
|
|||
|
||||
import libvirt
|
||||
import mock
|
||||
import threading
|
||||
|
||||
from pyghmi.ipmi import bmc
|
||||
from virtualbmc.tests.unit import base
|
||||
from virtualbmc.tests.unit import utils as test_utils
|
||||
from virtualbmc import utils
|
||||
|
|
@ -261,263 +259,3 @@ class VirtualBMCTestCase(base.TestCase):
|
|||
self.assertEqual(0xC0, ret)
|
||||
self.assertFalse(mock_libvirt_domain.return_value.create.called)
|
||||
self._assert_libvirt_calls(mock_libvirt_domain, mock_libvirt_open)
|
||||
|
||||
def test_is_active_active(self, mock_libvirt_domain, mock_libvirt_open):
|
||||
domain = mock_libvirt_domain.return_value
|
||||
domain.isActive.return_value = True
|
||||
ret = self.vbmc.is_active()
|
||||
|
||||
# power is already on, assert create() wasn't invoked
|
||||
self.assertEqual(ret, True)
|
||||
self._assert_libvirt_calls(mock_libvirt_domain, mock_libvirt_open)
|
||||
|
||||
def test_is_active_inactive(self, mock_libvirt_domain, mock_libvirt_open):
|
||||
domain = mock_libvirt_domain.return_value
|
||||
domain.isActive.return_value = False
|
||||
ret = self.vbmc.is_active()
|
||||
|
||||
self.assertEqual(ret, False)
|
||||
self._assert_libvirt_calls(mock_libvirt_domain, mock_libvirt_open)
|
||||
|
||||
def test_is_active_error(self, mock_libvirt_domain, mock_libvirt_open):
|
||||
mock_libvirt_domain.side_effect = libvirt.libvirtError('boom')
|
||||
ret = self.vbmc.is_active()
|
||||
self.assertEqual(ret, None)
|
||||
self._assert_libvirt_calls(mock_libvirt_domain, mock_libvirt_open)
|
||||
|
||||
def test_iohandler(self, mock_libvirt_domain, mock_libvirt_open):
|
||||
mock_stream = mock.MagicMock()
|
||||
self.vbmc._stream = mock_stream
|
||||
self.vbmc.iohandler('foo')
|
||||
mock_stream.send.assert_called_with('foo')
|
||||
|
||||
def test_iohandler_empty_stream(self, mock_libvirt_domain,
|
||||
mock_libvirt_open):
|
||||
self.vbmc._stream = None
|
||||
self.vbmc.iohandler('foo')
|
||||
|
||||
def test_check_console(self, mock_libvirt_domain, mock_libvirt_open):
|
||||
mock_conn = mock.MagicMock()
|
||||
mock_domain = mock.MagicMock()
|
||||
self.vbmc._conn = mock_conn
|
||||
self.vbmc._domain = mock_domain
|
||||
self.vbmc._run_console = True
|
||||
self.vbmc._state = [libvirt.VIR_DOMAIN_RUNNING]
|
||||
self.vbmc._stream = None
|
||||
ret = self.vbmc.check_console()
|
||||
|
||||
mock_conn.newStream.assert_called()
|
||||
mock_domain.openConsole.assert_called()
|
||||
mock_stream = mock_conn.newStream.return_value
|
||||
mock_stream.eventAddCallback.assert_called()
|
||||
mock_stream.eventRemoveCallback.assert_not_called()
|
||||
self.assertEqual(ret, True)
|
||||
|
||||
def test_check_console_stream(self, mock_libvirt_domain,
|
||||
mock_libvirt_open):
|
||||
mock_conn = mock.MagicMock()
|
||||
mock_domain = mock.MagicMock()
|
||||
mock_stream = mock.MagicMock()
|
||||
self.vbmc._conn = mock_conn
|
||||
self.vbmc._domain = mock_domain
|
||||
self.vbmc._run_console = True
|
||||
self.vbmc._state = [libvirt.VIR_DOMAIN_RUNNING]
|
||||
self.vbmc._stream = mock_stream
|
||||
ret = self.vbmc.check_console()
|
||||
|
||||
mock_conn.newStream.assert_not_called()
|
||||
mock_domain.openConsole.assert_not_called()
|
||||
mock_stream.eventAddCallback.assert_not_called()
|
||||
mock_stream.eventRemoveCallback.assert_not_called()
|
||||
self.assertEqual(ret, True)
|
||||
|
||||
def test_check_console_shutoff(self, mock_libvirt_domain,
|
||||
mock_libvirt_open):
|
||||
mock_conn = mock.MagicMock()
|
||||
mock_domain = mock.MagicMock()
|
||||
self.vbmc._conn = mock_conn
|
||||
self.vbmc._domain = mock_domain
|
||||
self.vbmc._run_console = True
|
||||
self.vbmc._state = [libvirt.VIR_DOMAIN_SHUTOFF]
|
||||
self.vbmc._stream = None
|
||||
ret = self.vbmc.check_console()
|
||||
|
||||
mock_conn.newStream.assert_not_called()
|
||||
mock_domain.openConsole.assert_not_called()
|
||||
self.assertEqual(ret, True)
|
||||
|
||||
def test_check_console_shutoff_stream(self, mock_libvirt_domain,
|
||||
mock_libvirt_open):
|
||||
mock_conn = mock.MagicMock()
|
||||
mock_domain = mock.MagicMock()
|
||||
mock_stream = mock.MagicMock()
|
||||
self.vbmc._conn = mock_conn
|
||||
self.vbmc._domain = mock_domain
|
||||
self.vbmc._run_console = True
|
||||
self.vbmc._state = [libvirt.VIR_DOMAIN_SHUTOFF]
|
||||
self.vbmc._stream = mock_stream
|
||||
ret = self.vbmc.check_console()
|
||||
|
||||
mock_conn.newStream.assert_not_called()
|
||||
mock_domain.openConsole.assert_not_called()
|
||||
mock_stream.eventRemoveCallback.assert_called()
|
||||
self.assertEqual(ret, True)
|
||||
|
||||
def test_check_console_none(self, mock_libvirt_domain,
|
||||
mock_libvirt_open):
|
||||
mock_conn = mock.MagicMock()
|
||||
mock_domain = mock.MagicMock()
|
||||
mock_stream = mock.MagicMock()
|
||||
self.vbmc._conn = mock_conn
|
||||
self.vbmc._domain = mock_domain
|
||||
self.vbmc._run_console = True
|
||||
self.vbmc._state = None
|
||||
self.vbmc._stream = None
|
||||
ret = self.vbmc.check_console()
|
||||
|
||||
mock_conn.newStream.assert_not_called()
|
||||
mock_domain.openConsole.assert_not_called()
|
||||
mock_stream.eventAddCallback.assert_not_called()
|
||||
mock_stream.eventRemoveCallback.assert_not_called()
|
||||
self.assertEqual(ret, True)
|
||||
|
||||
@mock.patch.object(threading.Thread, 'start')
|
||||
@mock.patch.object(bmc.Bmc, 'activate_payload')
|
||||
@mock.patch.object(bmc.Bmc, 'deactivate_payload')
|
||||
def test_activate_payload_deactivated(self,
|
||||
mock_deactivate_payload,
|
||||
mock_activate_payload,
|
||||
mock_thread_start,
|
||||
mock_libvirt_domain,
|
||||
mock_libvirt_open):
|
||||
self.vbmc.activated = False
|
||||
self.vbmc.activate_payload('foo', 'bar')
|
||||
|
||||
mock_thread_start.assert_called()
|
||||
mock_activate_payload.assert_called_with('foo', 'bar')
|
||||
mock_deactivate_payload.assert_not_called()
|
||||
self._assert_libvirt_calls(mock_libvirt_domain, mock_libvirt_open)
|
||||
|
||||
@mock.patch.object(threading.Thread, 'start')
|
||||
@mock.patch.object(bmc.Bmc, 'activate_payload')
|
||||
@mock.patch.object(bmc.Bmc, 'deactivate_payload')
|
||||
def test_activate_payload_activated(self,
|
||||
mock_deactivate_payload,
|
||||
mock_activate_payload,
|
||||
mock_thread_start,
|
||||
mock_libvirt_domain,
|
||||
mock_libvirt_open):
|
||||
self.vbmc.activated = True
|
||||
self.vbmc.activate_payload('foo', 'bar')
|
||||
|
||||
mock_thread_start.assert_not_called()
|
||||
mock_activate_payload.assert_not_called()
|
||||
mock_deactivate_payload.assert_not_called()
|
||||
|
||||
@mock.patch.object(threading.Thread, 'start')
|
||||
@mock.patch.object(bmc.Bmc, 'activate_payload')
|
||||
@mock.patch.object(bmc.Bmc, 'deactivate_payload')
|
||||
def test_activate_payload_error(self,
|
||||
mock_deactivate_payload,
|
||||
mock_activate_payload,
|
||||
mock_thread_start,
|
||||
mock_libvirt_domain,
|
||||
mock_libvirt_open):
|
||||
mock_libvirt_domain.side_effect = libvirt.libvirtError('boom')
|
||||
self.vbmc.activated = False
|
||||
self.vbmc.activate_payload('foo', 'bar')
|
||||
|
||||
mock_thread_start.assert_not_called()
|
||||
mock_activate_payload.assert_called_with('foo', 'bar')
|
||||
mock_deactivate_payload.assert_called_with('foo', 'bar')
|
||||
self._assert_libvirt_calls(mock_libvirt_domain, mock_libvirt_open)
|
||||
|
||||
@mock.patch.object(bmc.Bmc, 'deactivate_payload')
|
||||
def test_deactivate_payload_activated(self,
|
||||
mock_deactivate_payload,
|
||||
mock_libvirt_domain,
|
||||
mock_libvirt_open):
|
||||
mock_conn = mock.MagicMock()
|
||||
mock_thread = mock.MagicMock()
|
||||
self.vbmc.activated = True
|
||||
self.vbmc._conn = mock_conn
|
||||
self.vbmc._sol_thread = mock_thread
|
||||
self.vbmc.deactivate_payload('foo', 'bar')
|
||||
|
||||
mock_thread.join.assert_called()
|
||||
mock_conn.close.assert_called()
|
||||
mock_deactivate_payload.assert_called_with('foo', 'bar')
|
||||
|
||||
@mock.patch.object(bmc.Bmc, 'deactivate_payload')
|
||||
def test_deactivate_payload_deactivated(self,
|
||||
mock_deactivate_payload,
|
||||
mock_libvirt_domain,
|
||||
mock_libvirt_open):
|
||||
mock_conn = mock.MagicMock()
|
||||
mock_thread = mock.MagicMock()
|
||||
self.vbmc._activated = False
|
||||
self.vbmc._conn = mock_conn
|
||||
self.vbmc._sol_thread = mock_thread
|
||||
self.vbmc.deactivate_payload('foo', 'bar')
|
||||
|
||||
mock_thread.join.assert_not_called()
|
||||
mock_conn.close.assert_not_called()
|
||||
mock_deactivate_payload.assert_not_called()
|
||||
|
||||
@mock.patch.object(bmc.Bmc, 'deactivate_payload')
|
||||
def test_deactivate_payload_error(self,
|
||||
mock_deactivate_payload,
|
||||
mock_libvirt_domain,
|
||||
mock_libvirt_open):
|
||||
mock_conn = mock.MagicMock()
|
||||
mock_thread = mock.MagicMock()
|
||||
mock_thread.join.side_effect = libvirt.libvirtError('boom')
|
||||
self.vbmc.activated = True
|
||||
self.vbmc._conn = mock_conn
|
||||
self.vbmc._sol_thread = mock_thread
|
||||
self.vbmc.deactivate_payload('foo', 'bar')
|
||||
|
||||
mock_thread.join.assert_called()
|
||||
mock_conn.close.assert_called()
|
||||
mock_deactivate_payload.assert_not_called()
|
||||
|
||||
def test_lifecycle_callback(self, mock_libvirt_domain, mock_libvirt_open):
|
||||
mock_domain = mock.MagicMock()
|
||||
mock_domain.state.return_value = [libvirt.VIR_DOMAIN_RUNNING]
|
||||
self.vbmc._state = None
|
||||
self.vbmc._domain = mock_domain
|
||||
vbmc.lifecycle_callback(None, None, None, None, self.vbmc)
|
||||
self.assertEqual(self.vbmc.state, [libvirt.VIR_DOMAIN_RUNNING])
|
||||
|
||||
@mock.patch.object(vbmc.LOG, 'error')
|
||||
def test_error_handler_ignore(self, mock_error, mock_libvirt_domain,
|
||||
mock_libvirt_open):
|
||||
vbmc.error_handler(None,
|
||||
(libvirt.VIR_ERR_RPC, libvirt.VIR_FROM_STREAMS))
|
||||
mock_error.assert_not_called()
|
||||
|
||||
@mock.patch.object(vbmc.LOG, 'error')
|
||||
def test_error_handler_error(self, mock_error, mock_libvirt_domain,
|
||||
mock_libvirt_open):
|
||||
vbmc.error_handler(None,
|
||||
(libvirt.VIR_ERR_ERROR, libvirt.VIR_FROM_STREAMS))
|
||||
mock_error.assert_called()
|
||||
|
||||
def test_stream_callback(self, mock_libvirt_domain, mock_libvirt_open):
|
||||
mock_stream = mock.MagicMock()
|
||||
mock_sol = mock.MagicMock()
|
||||
mock_stream.recv.return_value = 'foo'
|
||||
self.vbmc.sol = mock_sol
|
||||
self.vbmc._stream = mock_stream
|
||||
vbmc.stream_callback(None, None, self.vbmc)
|
||||
mock_sol.send_data.assert_called_with('foo')
|
||||
|
||||
def test_stream_callback_error(self, mock_libvirt_domain,
|
||||
mock_libvirt_open):
|
||||
mock_stream = mock.MagicMock()
|
||||
mock_stream.recv.side_effect = libvirt.libvirtError('boom')
|
||||
mock_sol = mock.MagicMock()
|
||||
self.vbmc.sol = mock_sol
|
||||
self.vbmc._stream = mock_stream
|
||||
vbmc.stream_callback(None, None, self.vbmc)
|
||||
mock_sol.send_data.assert_not_called()
|
||||
|
|
|
|||
|
|
@ -25,9 +25,6 @@ class libvirt_open(object):
|
|||
self.sasl_password = sasl_password
|
||||
self.readonly = readonly
|
||||
|
||||
def get_conn(self):
|
||||
return self.__enter__()
|
||||
|
||||
def __enter__(self):
|
||||
try:
|
||||
if self.sasl_username and self.sasl_password:
|
||||
|
|
|
|||
|
|
@ -10,7 +10,6 @@
|
|||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import threading
|
||||
import xml.etree.ElementTree as ET
|
||||
|
||||
import libvirt
|
||||
|
|
@ -47,29 +46,6 @@ SET_BOOT_DEVICES_MAP = {
|
|||
'optical': 'cdrom',
|
||||
}
|
||||
|
||||
VIR_DOMAIN_ALIVE = [libvirt.VIR_DOMAIN_RUNNING, libvirt.VIR_DOMAIN_PAUSED]
|
||||
|
||||
|
||||
def lifecycle_callback(connection, domain, event, detail, console):
|
||||
console.state = console._domain.state(0)
|
||||
|
||||
|
||||
def error_handler(unused, error):
|
||||
# The console stream errors on VM shutdown; we don't care
|
||||
if (error[0] == libvirt.VIR_ERR_RPC and
|
||||
error[1] == libvirt.VIR_FROM_STREAMS):
|
||||
return
|
||||
LOG.error('Error: %s %s', error[0], error[1])
|
||||
|
||||
|
||||
def stream_callback(stream, events, console):
|
||||
try:
|
||||
data = console._stream.recv(1024)
|
||||
if console.sol:
|
||||
console.sol.send_data(data)
|
||||
except Exception:
|
||||
return
|
||||
|
||||
|
||||
class VirtualBMC(bmc.Bmc):
|
||||
|
||||
|
|
@ -83,15 +59,6 @@ class VirtualBMC(bmc.Bmc):
|
|||
'sasl_username': libvirt_sasl_username,
|
||||
'sasl_password': libvirt_sasl_password}
|
||||
|
||||
self._domain = None
|
||||
self._state = None
|
||||
self._stream = None
|
||||
self._run_console = False
|
||||
self._sol_thread = None
|
||||
|
||||
libvirt.virEventRegisterDefaultImpl()
|
||||
libvirt.registerErrorHandler(error_handler, None)
|
||||
|
||||
def get_boot_device(self):
|
||||
LOG.debug('Get boot device called for %s', self.domain_name)
|
||||
with utils.libvirt_open(readonly=True, **self._conn_args) as conn:
|
||||
|
|
@ -224,93 +191,5 @@ class VirtualBMC(bmc.Bmc):
|
|||
LOG.error('Error reseting the domain %(domain)s. '
|
||||
'Error: %(error)s' % {'domain': self.domain_name,
|
||||
'error': e})
|
||||
# Command failed, but let client to retry
|
||||
# Command not supported in present state
|
||||
return IPMI_COMMAND_NODE_BUSY
|
||||
|
||||
def is_active(self):
|
||||
try:
|
||||
with utils.libvirt_open(**self._conn_args) as conn:
|
||||
domain = utils.get_libvirt_domain(conn, self.domain_name)
|
||||
return domain.isActive()
|
||||
except libvirt.libvirtError as e:
|
||||
LOG.error('Error checking domain %(domain)s is alive. '
|
||||
'Error: %(error)s' % {'domain': self.domain_name,
|
||||
'error': e})
|
||||
|
||||
def iohandler(self, data):
|
||||
if self._stream:
|
||||
self._stream.send(data)
|
||||
|
||||
def loop(self):
|
||||
while self.check_console():
|
||||
libvirt.virEventRunDefaultImpl()
|
||||
|
||||
def check_console(self):
|
||||
if self._state is not None and self._state[0] in VIR_DOMAIN_ALIVE:
|
||||
if self._stream is None:
|
||||
self._stream = self._conn.newStream(
|
||||
libvirt.VIR_STREAM_NONBLOCK)
|
||||
self._domain.openConsole(None, self._stream, 0)
|
||||
self._stream.eventAddCallback(
|
||||
libvirt.VIR_STREAM_EVENT_READABLE, stream_callback, self)
|
||||
else:
|
||||
if self._stream:
|
||||
self._stream.eventRemoveCallback()
|
||||
self._stream = None
|
||||
|
||||
return self._run_console
|
||||
|
||||
def activate_payload(self, request, session):
|
||||
"""Connect VM serial console to the IPMI session
|
||||
|
||||
:param request: IPMI request packet
|
||||
:type request: dict
|
||||
:param session: IPMI session
|
||||
:type session: ServerSession object
|
||||
:rtype: None
|
||||
"""
|
||||
LOG.debug('Activate payload called for domain %s', self.domain_name)
|
||||
if self.activated:
|
||||
LOG.error('Error activating payload the domain %(domain)s. '
|
||||
'Error: Payload already activated' % {
|
||||
'domain': self.domain_name})
|
||||
return
|
||||
super(VirtualBMC, self).activate_payload(request, session)
|
||||
try:
|
||||
self._conn = utils.libvirt_open(**self._conn_args).get_conn()
|
||||
self._conn.domainEventRegister(lifecycle_callback, self)
|
||||
self._domain = utils.get_libvirt_domain(self._conn,
|
||||
self.domain_name)
|
||||
self._state = self._domain.state(0)
|
||||
self._run_console = True
|
||||
self._sol_thread = threading.Thread(target=self.loop)
|
||||
self._sol_thread.start()
|
||||
except libvirt.libvirtError as e:
|
||||
LOG.error('Error activating payload the domain %(domain)s. '
|
||||
'Error: %(error)s' % {'domain': self.domain_name,
|
||||
'error': e})
|
||||
super(VirtualBMC, self).deactivate_payload(request, session)
|
||||
|
||||
def deactivate_payload(self, request, session):
|
||||
"""Disonnect VM serial console from the IPMI session
|
||||
|
||||
:param request: IPMI request packet
|
||||
:type request: dict
|
||||
:param session: IPMI session
|
||||
:type session: ServerSession object
|
||||
:rtype: None
|
||||
"""
|
||||
LOG.debug('Deactivate payload called for domain %s', self.domain_name)
|
||||
if not self.activated:
|
||||
LOG.debug('Payload already deactivated')
|
||||
return
|
||||
try:
|
||||
self._run_console = False
|
||||
self._sol_thread.join()
|
||||
super(VirtualBMC, self).deactivate_payload(request, session)
|
||||
except libvirt.libvirtError as e:
|
||||
LOG.error('Error deactivating payload the domain %(domain)s. '
|
||||
'Error: %(error)s' % {'domain': self.domain_name,
|
||||
'error': e})
|
||||
finally:
|
||||
self._conn.close()
|
||||
|
|
|
|||
Loading…
Reference in New Issue