mooli tayer has uploaded a new change for review. Change subject: Decoupling libvirtconnection.py from clintIf.py ......................................................................
Decoupling libvirtconnection.py from clintIf.py 1.) Decoupling libvirtconnection.py from clientIF.py adding libvirt cb handlers for vm events belongs in vm.py 2.) bugfix: not all libviryconnecition.get() invocations from vdsm are wrapped with error handling code (with an important implication in the bug description: causes vdsm to shutdown on libvirt connection error) These two come in the same patch because removing the cli dependency (clientIF instance) from libvirtconnection must handle errors in a new way - sendind SIGTERM to the current process which solves the bug. Change-Id: Ibc550ea40ebb3abcf37ebfa59cbf8df8e42daa96 Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=958367 Signed-off-by: Mooli Tayer <[email protected]> --- M lib/vdsm/libvirtconnection.py M lib/vdsm/tool/dummybr.py M lib/vdsm/tool/nwfilter.py M tests/vmTests.py M vdsm/vm.py 5 files changed, 87 insertions(+), 71 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/93/16393/1 diff --git a/lib/vdsm/libvirtconnection.py b/lib/vdsm/libvirtconnection.py index 2c2bfb3..18480f2 100644 --- a/lib/vdsm/libvirtconnection.py +++ b/lib/vdsm/libvirtconnection.py @@ -21,6 +21,7 @@ import threading import functools import logging +import os import libvirt from vdsm import constants, utils @@ -53,58 +54,16 @@ __event_loop.stop() -def __eventCallback(conn, dom, *args): - try: - cif, eventid = args[-1] - vmid = dom.UUIDString() - v = cif.vmContainer.get(vmid) - - if not v: - cif.log.debug('unknown vm %s eventid %s args %s', - vmid, eventid, args) - return - - if eventid == libvirt.VIR_DOMAIN_EVENT_ID_LIFECYCLE: - event, detail = args[:-1] - v._onLibvirtLifecycleEvent(event, detail, None) - elif eventid == libvirt.VIR_DOMAIN_EVENT_ID_REBOOT: - v.onReboot() - elif eventid == libvirt.VIR_DOMAIN_EVENT_ID_RTC_CHANGE: - utcoffset, = args[:-1] - v._rtcUpdate(utcoffset) - elif eventid == libvirt.VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON: - srcPath, devAlias, action, reason = args[:-1] - v._onAbnormalStop(devAlias, reason) - elif eventid == libvirt.VIR_DOMAIN_EVENT_ID_GRAPHICS: - phase, localAddr, remoteAddr, authScheme, subject = args[:-1] - v.log.debug('graphics event phase %s localAddr %s remoteAddr %s' - 'authScheme %s subject %s', - phase, localAddr, remoteAddr, authScheme, subject) - if phase == libvirt.VIR_DOMAIN_EVENT_GRAPHICS_INITIALIZE: - v.onConnect(remoteAddr['node']) - elif phase == libvirt.VIR_DOMAIN_EVENT_GRAPHICS_DISCONNECT: - v.onDisconnect() - elif eventid == libvirt.VIR_DOMAIN_EVENT_ID_BLOCK_JOB: - path, type, status = args[:-1] - v._onBlockJobEvent(path, type, status) - elif eventid == libvirt.VIR_DOMAIN_EVENT_ID_WATCHDOG: - action, = args[:-1] - v._onWatchdogEvent(action) - else: - v.log.warning('unknown eventid %s args %s', eventid, args) - except: - cif.log.error("Error running VM callback", exc_info=True) - - __connections = {} __connectionLock = threading.Lock() -def get(cif=None): +def get(target=None, killOnFailure=True): """Return current connection to libvirt or open a new one. + Use target to get/create the connection object linked to that object. Wrap methods of connection object so that they catch disconnection, and - take vdsm down. + take the current process down. """ def wrapMethod(f): def wrapper(*args, **kwargs): @@ -117,6 +76,10 @@ setattr(ret, name, wrapMethod(method)) return ret except libvirt.libvirtError as e: + if target is not None and hasattr(target, 'log'): + log = target.log + else: + log = logging.getLogger() edom = e.get_error_domain() ecode = e.get_error_code() EDOMAINS = (libvirt.VIR_FROM_REMOTE, @@ -126,14 +89,15 @@ libvirt.VIR_ERR_NO_CONNECT, libvirt.VIR_ERR_INVALID_CONN) if edom in EDOMAINS and ecode in ECODES: - cif.log.error('connection to libvirt broken. ' - 'taking vdsm down. ecode: %d edom: %d', - ecode, edom) - cif.prepareForShutdown() + log.error('connection to libvirt broken. ') + if killOnFailure: + log.error('taking calling process down. ecode:' + ' %d edom: %d', ecode, edom) + os.kill(os.getpid(), 15) else: - cif.log.debug('Unknown libvirterror: ecode: %d edom: %d ' - 'level: %d message: %s', ecode, edom, - e.get_error_level(), e.get_error_message()) + 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__ @@ -152,27 +116,18 @@ req, None] with __connectionLock: - conn = __connections.get(id(cif)) + conn = __connections.get(id(target)) if not conn: libvirtOpenAuth = functools.partial(libvirt.openAuth, 'qemu:///system', auth, 0) logging.debug('trying to connect libvirt') conn = utils.retry(libvirtOpenAuth, timeout=10, sleep=0.2) - __connections[id(cif)] = conn - if cif is not None: - for ev in (libvirt.VIR_DOMAIN_EVENT_ID_LIFECYCLE, - libvirt.VIR_DOMAIN_EVENT_ID_REBOOT, - libvirt.VIR_DOMAIN_EVENT_ID_RTC_CHANGE, - libvirt.VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON, - libvirt.VIR_DOMAIN_EVENT_ID_GRAPHICS, - libvirt.VIR_DOMAIN_EVENT_ID_BLOCK_JOB, - libvirt.VIR_DOMAIN_EVENT_ID_WATCHDOG): - conn.domainEventRegisterAny(None, ev, - __eventCallback, (cif, ev)) - for name in dir(libvirt.virConnect): - method = getattr(conn, name) - if callable(method) and name[0] != '_': - setattr(conn, name, wrapMethod(method)) + __connections[id(target)] = conn + + for name in dir(libvirt.virConnect): + method = getattr(conn, name) + if callable(method) and name[0] != '_': + setattr(conn, name, wrapMethod(method)) # In case we're running into troubles with keeping the connections # alive we should place here: # conn.setKeepAlive(interval=5, count=3) diff --git a/lib/vdsm/tool/dummybr.py b/lib/vdsm/tool/dummybr.py index bc47fbb..762e726 100644 --- a/lib/vdsm/tool/dummybr.py +++ b/lib/vdsm/tool/dummybr.py @@ -34,7 +34,7 @@ def addBridgeToLibvirt(bridgeName): - conn = libvirtconnection.get() + conn = libvirtconnection.get(None, False) if bridgeName not in conn.listNetworks(): conn.networkCreateXML( '''<network><name>%s</name><forward mode='bridge'/><bridge ''' diff --git a/lib/vdsm/tool/nwfilter.py b/lib/vdsm/tool/nwfilter.py index 85d9dff..54bbe8d 100755 --- a/lib/vdsm/tool/nwfilter.py +++ b/lib/vdsm/tool/nwfilter.py @@ -32,7 +32,7 @@ """ Defines network filters on libvirt """ - conn = libvirtconnection.get() + conn = libvirtconnection.get(None, False) NoMacSpoofingFilter().defineNwFilter(conn) conn.close() diff --git a/tests/vmTests.py b/tests/vmTests.py index 15b1ba9..4ba6208 100644 --- a/tests/vmTests.py +++ b/tests/vmTests.py @@ -32,6 +32,12 @@ from vmTestsData import CONF_TO_DOMXML +class connectionMock: + def domainEventRegisterAny(self, *arg): + pass +cm = connectionMock() + + class TestVm(TestCaseBase): PCI_ADDR = \ @@ -480,7 +486,7 @@ 'release': '1', 'version': '18', 'name': 'Fedora'}) @MonkeyPatch(constants, 'SMBIOS_MANUFACTURER', 'oVirt') @MonkeyPatch(constants, 'SMBIOS_OSNAME', 'oVirt Node') - @MonkeyPatch(libvirtconnection, 'get', lambda x: 0) + @MonkeyPatch(libvirtconnection, 'get', lambda x: cm) @MonkeyPatch(utils, 'getHostUUID', lambda: "fc25cbbe-5520-4f83-b82e-1541914753d9") def testBuildCmdLine(self): diff --git a/vdsm/vm.py b/vdsm/vm.py index e8118a3..6f96135 100644 --- a/vdsm/vm.py +++ b/vdsm/vm.py @@ -1667,6 +1667,18 @@ SMARTCARD_DEVICES: []} self._connection = libvirtconnection.get(cif) + for ev in (libvirt.VIR_DOMAIN_EVENT_ID_LIFECYCLE, + libvirt.VIR_DOMAIN_EVENT_ID_REBOOT, + libvirt.VIR_DOMAIN_EVENT_ID_RTC_CHANGE, + libvirt.VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON, + libvirt.VIR_DOMAIN_EVENT_ID_GRAPHICS, + libvirt.VIR_DOMAIN_EVENT_ID_BLOCK_JOB, + libvirt.VIR_DOMAIN_EVENT_ID_WATCHDOG): + self._connection.domainEventRegisterAny(None, + ev, + _dispatchLibvirtEvent, + (cif, ev)) + if 'vmName' not in self.conf: self.conf['vmName'] = 'n%s' % self.id self._guestSocketFile = self._makeChannelPath(_VMCHANNEL_DEVICE_NAME) @@ -4552,6 +4564,49 @@ return True +def _dispatchLibvirtEvent(conn, dom, *args): + try: + cif, eventid = args[-1] + vmid = dom.UUIDString() + v = cif.vmContainer.get(vmid) + + if not v: + cif.log.debug('unknown vm %s eventid %s args %s', + vmid, eventid, args) + return + + if eventid == libvirt.VIR_DOMAIN_EVENT_ID_LIFECYCLE: + event, detail = args[:-1] + v._onLibvirtLifecycleEvent(event, detail, None) + elif eventid == libvirt.VIR_DOMAIN_EVENT_ID_REBOOT: + v.onReboot() + elif eventid == libvirt.VIR_DOMAIN_EVENT_ID_RTC_CHANGE: + utcoffset, = args[:-1] + v._rtcUpdate(utcoffset) + elif eventid == libvirt.VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON: + srcPath, devAlias, action, reason = args[:-1] + v._onAbnormalStop(devAlias, reason) + elif eventid == libvirt.VIR_DOMAIN_EVENT_ID_GRAPHICS: + phase, localAddr, remoteAddr, authScheme, subject = args[:-1] + v.log.debug('graphics event phase %s localAddr %s remoteAddr %s' + 'authScheme %s subject %s', + phase, localAddr, remoteAddr, authScheme, subject) + if phase == libvirt.VIR_DOMAIN_EVENT_GRAPHICS_INITIALIZE: + v.onConnect(remoteAddr['node']) + elif phase == libvirt.VIR_DOMAIN_EVENT_GRAPHICS_DISCONNECT: + v.onDisconnect() + elif eventid == libvirt.VIR_DOMAIN_EVENT_ID_BLOCK_JOB: + path, type, status = args[:-1] + v._onBlockJobEvent(path, type, status) + elif eventid == libvirt.VIR_DOMAIN_EVENT_ID_WATCHDOG: + action, = args[:-1] + v._onWatchdogEvent(action) + else: + v.log.warning('unknown eventid %s args %s', eventid, args) + except: + cif.log.error("Error running VM callback", exc_info=True) + + # A little unrelated hack to make xml.dom.minidom.Document.toprettyxml() # not wrap Text node with whitespace. # until http://bugs.python.org/issue4147 is accepted -- To view, visit http://gerrit.ovirt.org/16393 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ibc550ea40ebb3abcf37ebfa59cbf8df8e42daa96 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
