Antoni Segura Puimedon has posted comments on this change. Change subject: net_scale: avoid netinfo instantiation for removal check ......................................................................
Patch Set 2: (2 comments) http://gerrit.ovirt.org/#/c/24158/2/lib/vdsm/netinfo.py File lib/vdsm/netinfo.py: Line 22: import errno Line 23: from glob import iglob Line 24: from datetime import datetime Line 25: from itertools import chain Line 26: import libvirt > it's not in stdlib, so place near ethtool. Done Line 27: import logging Line 28: import os Line 29: import shlex Line 30: import socket http://gerrit.ovirt.org/#/c/24158/2/vdsm/configNetwork.py File vdsm/configNetwork.py: Line 412: # it still has users and thus does not allow its removal Line 413: configurator.removeLibvirtNetwork(network) Line 414: netEnt.remove() Line 415: Line 416: # We need to gather NetInfo again to refresh networks info from libvirt. > How about completely removing this costly assertion? Why not just trust con Well, currently it remains a case of "fail early, fail hard" so that if a lot of setupNetwork operations were to be handled afterwards they'd be aborted. But I agree that this failing should be raised by the configurators. Line 417: # The deleted net should never exist at this stage. Line 418: if netinfo.networkExists(network): Line 419: raise ConfigNetworkError(ne.ERR_USED_BRIDGE, 'delNetwork: bridge %s ' Line 420: 'still exists' % network) -- To view, visit http://gerrit.ovirt.org/24158 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1ff62dcd750ece1c3685c102723820aac5b432b2 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]> Gerrit-Reviewer: Ondřej Svoboda <[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
