Dan Kenigsberg 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.
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 
configurator.removeLibvirtNetwork(network) to do the thing it was designed for?

Douglas, do you recall if there's a particular importance for this 
http://gerrit.ovirt.org/4342 ?
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: 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

Reply via email to