Change in vdsm[master]: network: Introduce the network package

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: network: Introduce the network package
......................................................................

Patch Set 3:

(5 comments)

The libvirtCfg rename to libvirt worries me a little. Hopefully we won't get confused. Minor comments to be addressed/answered.

http://gerrit.ovirt.org/#/c/25908/3/vdsm/network/api.py
File vdsm/network/api.py:

Line 34: from .configurators import libvirt
Line 35: from .errors import ConfigNetworkError
Line 36: from . import errors as ne
Line 37: from .models import Bond, Bridge, IPv4, IPv6, IpConfig, Nic, Vlan
Line 38: import hooks  # TODO: This absolute import should go
This should probably go before line 28.
Line 39: 
Line 40: CONNECTIVITY_TIMEOUT_DEFAULT = 4
Line 41: 
Line 42:

Line 675: 
Line 676: def usage():
Line 677:     print """Usage:
Line 678:     ./api.py add Network <attributes> <options>
Line 679:                        edit oldNetwork newNetwork <attributes> <options>
These should be re-indented properly. Any plan (another patch) to move this utility out of the libraries?
Line 680:                        del Network <options>
Line 681:                        setup Network [None|attributes] \
Line 682: [++ Network [None|attributes] [++ ...]] [:: <options>]
Line 683:

http://gerrit.ovirt.org/#/c/25908/3/vdsm/network/configurators/ifcfg.py
File vdsm/network/configurators/ifcfg.py:

Line 16: #
Line 17: # Refer to the README and COPYING files for full details of the license
Line 18: #
Line 19: 
Line 20: from libvirt import libvirtError, VIR_ERR_NO_NETWORK
Since this is not part of python and it's a "from" import it probably looks better at line 30.
Line 21: import glob
Line 22: import logging
Line 23: import os
Line 24: import pipes

Line 38: from . import Configurator, getEthtoolOpts, libvirt
Line 39: from ..errors import ConfigNetworkError, ERR_FAILED_IFUP
Line 40: from ..models import Nic, Bridge, IpConfig
Line 41: from ..sourceroute import DynamicSourceRoute
Line 42: import dsaversion  # TODO: This absolute import from the package should go
This should probably go before line 31.
Line 43: 
Line 44: 
Line 45: def _hwaddr_required():
Line 46:     return config.get('vars', 'hwaddr_in_ifcfg') == 'always'

http://gerrit.ovirt.org/#/c/25908/3/vdsm/network/configurators/libvirt.py
File vdsm/network/configurators/libvirt.py:

Line 15: # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
Line 16: #
Line 17: # Refer to the README and COPYING files for full details of the license
Line 18: #
Line 19: from __future__ import absolute_import
Why is this needed here and not it ifcfg.py as well?
Line 20: 
Line 21: from xml.dom.minidom import Document
Line 22: from xml.sax.saxutils import escape
Line 23: