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