Nir Soffer has posted comments on this change. Change subject: External hypervisor VMs integration ......................................................................
Patch Set 12: Code-Review-1 (1 comment) Partial review - incorrect use of try-finally in v2v. http://gerrit.ovirt.org/#/c/33309/12/vdsm/v2v.py File vdsm/v2v.py: Line 110: Line 111: Line 112: def getExternalVMList(uri, username, password): Line 113: try: Line 114: conn = _hypervisorConnect(uri, username, password) If _hypervisorConnect fails, conn is not defined, and conn.close() in finally will fail with NameError. Should be: conn = _hypervisorConnect(...) try: ... finally: conn.close() Or better: from contextlib import closing ... conn = _hypervisorConnect(...) with closing(conn): ... Question: why we cannot use the current libvirt connection? This may create lot of short lived connections, without any limit on the number of concurrent connections. Line 115: if conn is None: Line 116: return None Line 117: Line 118: ret = [] -- To view, visit http://gerrit.ovirt.org/33309 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7dcfb860626a844d1d08590274b508519a33f4a3 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Shahar Havivi <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
