Change in vdsm[master]: net: ovs: remove initial nic ip, dont touch unchanged networks

osvoboda at redhat.com osvoboda at redhat.com
Tue Oct 27 00:33:27 UTC 2015


Ondřej Svoboda has posted comments on this change.

Change subject: net: ovs: remove initial nic ip, dont touch unchanged networks
......................................................................


Patch Set 6: Code-Review-1

(7 comments)

Coding style pointers ;-)

https://gerrit.ovirt.org/#/c/46692/6/vdsm_hooks/ovs/ovs_before_network_setup_ip.py
File vdsm_hooks/ovs/ovs_before_network_setup_ip.py:

Line 118: top_dev, ipv4, ipv6, port, blocking_dhcp
Tuples are not bad by themselves, but I'd like to see a container object with named fields here (namedtuple for a start? but a class might be useful later).


Line 132: \
We can do without slashes.


Line 136: init_nets[net].get('nic') != attrs.get('nic') is not None
I failed to interpret this.


Line 139: net top
a network's top device?


Line 140: set the new one if any
Please be more verbose. Is it "any configuration"?


Line 149: if attrs.get('nic') is not None:
        :                     _drop_ip_config(attrs.get('nic'))
You can attrs.get('nic') first, store it in a variable and then see if it is not None (or even not empty, there should be some validation…)


Line 154: (ipv4, ipv6, port,
        :                                                  blocking_dhcp)
Again, tuples.


-- 
To view, visit https://gerrit.ovirt.org/46692
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9682c75cfb79f75b64cb8cb587fc3ed01cf15c8
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček <phoracek at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Jenkins CI
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-HasComments: Yes


More information about the vdsm-patches mailing list