mooli tayer has uploaded a new change for review.
Change subject: libvirtconnection: ping libvirt upon disconnection ......................................................................
libvirtconnection: ping libvirt upon disconnection
Upon recieving an error from libvirt we should ping it (using getLibVersion) and only upon failure treat it as down.
Libvirt error codes do not distinguish recoverable from non-recoverable libvirt errors.
Change-Id: Ia489e46dd8ce4c70c888988d17b86311d3c4b935 Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=982634 Signed-off-by: Mooli Tayer mtayer@redhat.com --- M lib/vdsm/libvirtconnection.py A tests/libvirtconnectionTests.py 2 files changed, 136 insertions(+), 13 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/44/19444/1
diff --git a/lib/vdsm/libvirtconnection.py b/lib/vdsm/libvirtconnection.py index cdba57c..b129c03 100644 --- a/lib/vdsm/libvirtconnection.py +++ b/lib/vdsm/libvirtconnection.py @@ -70,15 +70,15 @@ Wrap methods of connection object so that they catch disconnection, and take the current process down. """ - def wrapMethod(f): - def wrapper(*args, **kwargs): + def wrapMethod(conn, f): + def wrapper(vdsmLibvirtPing=False, *args, **kwargs): try: ret = f(*args, **kwargs) if isinstance(ret, libvirt.virDomain): for name in dir(ret): method = getattr(ret, name) if callable(method) and name[0] != '_': - setattr(ret, name, wrapMethod(method)) + setattr(ret, name, wrapMethod(conn, method)) return ret except libvirt.libvirtError as e: edom = e.get_error_domain() @@ -90,15 +90,19 @@ libvirt.VIR_ERR_NO_CONNECT, libvirt.VIR_ERR_INVALID_CONN) if edom in EDOMAINS and ecode in ECODES: - log.error('connection to libvirt broken.' - ' ecode: %d edom: %d', ecode, edom) - if killOnFailure: - log.error('taking calling process down.') - os.kill(os.getpid(), signal.SIGTERM) - else: - log.debug('Unknown libvirterror: ecode: %d edom: %d ' - 'level: %d message: %s', ecode, edom, - e.get_error_level(), e.get_error_message()) + if not vdsmLibvirtPing: + conn.getLibVersion(vdsmLibvirtPing=True) + else: + log.error('connection to libvirt broken.' + ' ecode: %d edom: %d', ecode, edom) + if killOnFailure: + log.error('taking calling process down.') + os.kill(os.getpid(), signal.SIGTERM) + else: + raise + log.debug('Unknown libvirterror: ecode: %d edom: %d ' + 'level: %d message: %s', ecode, edom, + e.get_error_level(), e.get_error_message()) raise wrapper.__name__ = f.__name__ wrapper.__doc__ = f.__doc__ @@ -128,7 +132,7 @@ for name in dir(libvirt.virConnect): method = getattr(conn, name) if callable(method) and name[0] != '_': - setattr(conn, name, wrapMethod(method)) + setattr(conn, name, wrapMethod(conn, method)) if target is not None: for ev in (libvirt.VIR_DOMAIN_EVENT_ID_LIFECYCLE, libvirt.VIR_DOMAIN_EVENT_ID_REBOOT, diff --git a/tests/libvirtconnectionTests.py b/tests/libvirtconnectionTests.py new file mode 100644 index 0000000..280dad9 --- /dev/null +++ b/tests/libvirtconnectionTests.py @@ -0,0 +1,119 @@ +# +# Copyright 2012 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +# 02110-1301 USA +# +# Refer to the README and COPYING files for full details of the license +# + +from vdsm import libvirtconnection +from testrunner import VdsmTestCase as TestCaseBase +import os +from monkeypatch import MonkeyPatch + + +class TerminationException(Exception): + pass + + +class LibvirtMock: + VIR_CRED_AUTHNAME = 1 + VIR_CRED_PASSPHRASE = 2 + VIR_FROM_RPC = 3 + VIR_FROM_REMOTE = 4 + VIR_ERR_SYSTEM_ERROR = 5 + VIR_ERR_INTERNAL_ERROR = 6 + VIR_ERR_NO_CONNECT = 7 + VIR_ERR_INVALID_CONN = 8 + + class libvirtError(Exception): + def get_error_code(self): + return LibvirtMock.VIR_ERR_SYSTEM_ERROR + + def get_error_domain(self): + return LibvirtMock.VIR_FROM_RPC + + def get_error_level(self): + return 3 + + def get_error_message(self): + return '' + + class virConnect: + failGetLibVersion = False + failNodeDeviceLookupByName = False + + def nodeDeviceLookupByName(self): + if LibvirtMock.virConnect.failNodeDeviceLookupByName: + print('nodeDeviceLookupByName called, exception') + + raise LibvirtMock.libvirtError() + else: + print('nodeDeviceLookupByName called, success') + return '' + + def getLibVersion(self): + if LibvirtMock.virConnect.failGetLibVersion: + print('getLibVersion called, exception') + raise LibvirtMock.libvirtError() + else: + print('getLibVersion called, success') + return '' + + class virDomain(): + pass + + def openAuth(self, *args): + return LibvirtMock.virConnect() + + +def _kill(*args): + raise TerminationException() + + +class testLibvirtconnection(TestCaseBase): + @MonkeyPatch(libvirtconnection, 'libvirt', LibvirtMock()) + def testCallSucceeded(self): + """Positive test - libvirtMock does not raise any errors""" + LibvirtMock.virConnect.failGetLibVersion = False + LibvirtMock.virConnect.failNodeDeviceLookupByName = False + connection = libvirtconnection.get() + connection.nodeDeviceLookupByName() + + @MonkeyPatch(libvirtconnection, 'libvirt', LibvirtMock()) + def testCallFailedConnectionUp(self): + """ + libvirtMock will raise an error when nodeDeviceLookupByName is called. + When getLibVersion is called (used by libvirtconnection to recognize disconnections) + it will not raise an error -> in that case an error should be raised ('Unknown libvirterror'). + """ + connection = libvirtconnection.get(killOnFailure=True) + LibvirtMock.virConnect.failNodeDeviceLookupByName = True + LibvirtMock.virConnect.failGetLibVersion = False + self.assertRaises(LibvirtMock.libvirtError, connection.nodeDeviceLookupByName) + + @MonkeyPatch(libvirtconnection, 'libvirt', LibvirtMock()) + @MonkeyPatch(os, 'kill', _kill) + def testCallFailedConnectionDown(self): + """ + libvirtMock will raise an error when nodeDeviceLookupByName is called. + When getLibVersion is called (used by libvirtconnection to recognize disconnections) + it will also raise an error -> in that case os.kill should be called('connection to libvirt broken.'). + """ + connection = libvirtconnection.get(killOnFailure=True) + LibvirtMock.virConnect.failNodeDeviceLookupByName = True + LibvirtMock.virConnect.failGetLibVersion = True + self.assertRaises(TerminationException, connection.nodeDeviceLookupByName) \ No newline at end of file