Change in vdsm[master]: netinfo: Modernize functions recognizing the usage of DHCP f...

danken at redhat.com danken at redhat.com
Fri Jan 9 09:37:31 UTC 2015


Dan Kenigsberg has posted comments on this change.

Change subject: netinfo: Modernize functions recognizing the usage of DHCP from lease files
......................................................................


Patch Set 9: Code-Review-1

(2 comments)

http://gerrit.ovirt.org/#/c/36038/9/lib/vdsm/netinfo.py
File lib/vdsm/netinfo.py:

Line 543: 
Line 544:     return dhcpv4_ifaces
Line 545: 
Line 546: 
Line 547: def _get_dhclient_ifaces(lease_files_globs):
you drop the ability to get ipv6 leases? is this intentional? Could this be explained in the commit message?
Line 548:     """Return a set of interfaces configured using dhclient.
Line 549: 
Line 550:     dhclient stores DHCP leases to file(s) whose names can be specified
Line 551:     by the lease_files_globs parameter (an iterable of glob strings).


Line 549: 
Line 550:     dhclient stores DHCP leases to file(s) whose names can be specified
Line 551:     by the lease_files_globs parameter (an iterable of glob strings).
Line 552:     """
Line 553:     dhcpv4_ifaces = set()
Please explain why you drop the TODO without adding new functionality
Line 554: 
Line 555:     for lease_files_glob in lease_files_globs:
Line 556:         for lease_path in iglob(lease_files_glob):
Line 557:             with open(lease_path) as lease_file:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5b95d0223b35c3bbbcd736c28ed1dd7c2f9bd2bb
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda 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