Change in vdsm[master]: IpConfig: Deal with missing parameters gracefully and pass t...

osvoboda at redhat.com osvoboda at redhat.com
Fri Jan 9 12:49:52 UTC 2015


Ondřej Svoboda has posted comments on this change.

Change subject: IpConfig: Deal with missing parameters gracefully and pass them as positional
......................................................................


Patch Set 4:

(1 comment)

Despite my explanation I am willing to split the renames from functional changes in the next patchset, of course :-)

http://gerrit.ovirt.org/#/c/36712/4/vdsm/network/models.py
File vdsm/network/models.py:

Line 478: 
Line 479:     def __init__(self, ipv4=None, ipv6=None, bootproto=None, blocking=None,
Line 480:                  ipv6autoconf=None, dhcpv6=None):
Line 481:         if ipv4 is None and ipv6 is None:
Line 482:             ipv4 = IPv4()
> please do not mix renaming of variable and functional changes. it makes it 
I know your general stance to mixing renames and functional changes but I took the liberty to rename the arguments because I would touch a substantial part of IpConfig's code and did not want to use the old names in getConfig again (in the rest of the codebase inet4 and inet6 are named ipv4 and ipv6 instead).

I will update the commit message in later stages of my topic but:

Today the only user of IpConfig.__init__ is network.api.objectivizeNetwork. I want to use IpConfig's attributes in the code directly, without the diversion through ipConfig namedtuple, which is really only needed in ifcfg configurator.

Another user of IpConfig.__init__ will be network.models.NetDevice. In its __init__ I will initialize its 'ip' member with IpConfig() so it will serve the same purpose as the namedtuple, but in a more clean way.

Please note that objectivizeNetwork always creates NetDevice subclasses without the ip/ipconfig member and only later it assigns IpConfig to topNetDev.

I have already pushed a patch that does with just IpConfig in iproute2 and pyroute_two configurators. And I am working on a slightly more complex refactor of the usage of IpConfig in ifcfg configurator so IpConfig and related code will become just clear.
Line 483:             ipv6 = IPv6()
Line 484:         elif ((ipv4.address and bootproto == 'dhcp') or
Line 485:               (ipv6.address and (ipv6autoconf or dhcpv6))):
Line 486:             raise ConfigNetworkError(ne.ERR_BAD_ADDR, 'Static and dynamic ip '


-- 
To view, visit http://gerrit.ovirt.org/36712
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I09a5d1e3468004f529c2bc9708cff2243c16241a
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <toni at midokura.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda at redhat.com>
Gerrit-Reviewer: Petr Horáček <phoracek at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list