Yaniv Bronhaim has posted comments on this change.
Change subject: libvirtconnection: ping libvirt upon disconnection ......................................................................
Patch Set 5:
(3 comments)
.................................................... File lib/vdsm/libvirtconnection.py Line 70: Wrap methods of connection object so that they catch disconnection, and Line 71: take the current process down. Line 72: """ Line 73: def wrapMethod(f): Line 74: def wrapper(_pingLibvirt=True, *args, **kwargs): underscore is to specified private variables in an object or globals. please omit it from this context Line 75: try: Line 76: ret = f(*args, **kwargs) Line 77: if isinstance(ret, libvirt.virDomain): Line 78: for name in dir(ret):
Line 89: libvirt.VIR_ERR_INTERNAL_ERROR, Line 90: libvirt.VIR_ERR_NO_CONNECT, Line 91: libvirt.VIR_ERR_INVALID_CONN) Line 92: if edom in EDOMAINS and ecode in ECODES: Line 93: if _pingLibvirt: Don't you agree with me that omitting this _pingLibvirt parameter that we pass to each method and instead not wrapping getLibVersion is better here? The only catch is if someone calls the getLibVersion and expects the wrapping flow, which doesn't really hurt here. The caller will see the regular exception. Line 94: conn = __connections.get(id(target)) Line 95: conn.getLibVersion(_pingLibvirt=False) Line 96: else: Line 97: log.error('connection to libvirt broken.'
Line 91: libvirt.VIR_ERR_INVALID_CONN) Line 92: if edom in EDOMAINS and ecode in ECODES: Line 93: if _pingLibvirt: Line 94: conn = __connections.get(id(target)) Line 95: conn.getLibVersion(_pingLibvirt=False) much better, even in one line: __connections.get(id(target)).getLibVersion(_pingLibvirt=False) Line 96: else: Line 97: log.error('connection to libvirt broken.' Line 98: ' ecode: %d edom: %d', ecode, edom) Line 99: if killOnFailure: