Yaniv Bronhaim has posted comments on this change.

Change subject: Decoupling libvirtconnection.py from clintIf.py
......................................................................


Patch Set 1: (3 inline comments)

....................................................
Commit Message
Line 13: (with an important implication in the bug description:
Line 14:  causes vdsm to shutdown on libvirt connection error)
Line 15: 
Line 16: These two come in the same patch because removing
Line 17: the cli dependency (clientIF instance) from libvirtconnection
you meant cif dependency..
Line 18: must handle errors in a new way - sendind SIGTERM to the current
Line 19: process which solves the bug.
Line 20: 
Line 21: Change-Id: Ibc550ea40ebb3abcf37ebfa59cbf8df8e42daa96


....................................................
File lib/vdsm/libvirtconnection.py
Line 78:             except libvirt.libvirtError as e:
Line 79:                 if target is not None and hasattr(target, 'log'):
Line 80:                     log = target.log
Line 81:                 else:
Line 82:                     log = logging.getLogger()
getLogger("libvirtconnection")
Line 83:                 edom = e.get_error_domain()
Line 84:                 ecode = e.get_error_code()
Line 85:                 EDOMAINS = (libvirt.VIR_FROM_REMOTE,
Line 86:                             libvirt.VIR_FROM_RPC)


Line 92:                     log.error('connection to libvirt broken. ')
Line 93:                     if killOnFailure:
Line 94:                         log.error('taking calling process down. ecode:'
Line 95:                                   ' %d edom: %d', ecode, edom)
Line 96:                         os.kill(os.getpid(), 15)
i think he did it in purpose. it doesn't chain the prints, and you'll have some 
of other prints between the 'connection to libvirt is broken' to this one. its 
better to copy that start to both log prints.
Line 97:                 else:
Line 98:                     log.debug('Unknown libvirterror: ecode: %d edom: 
%d '
Line 99:                               'level: %d message: %s', ecode, edom,
Line 100:                               e.get_error_level(), 
e.get_error_message())


-- 
To view, visit http://gerrit.ovirt.org/16393
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc550ea40ebb3abcf37ebfa59cbf8df8e42daa96
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer <mta...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to