Ondřej Svoboda has posted comments on this change.

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


Patch Set 9:

(2 comments)

Notes about obsolescence.

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
The ability was never there. I don't need the 'ipv6' parameter _parseLeaseFile 
in the actual implementation in a follow-up patch (though I will have to rebase 
it).
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
The TODO was mislead and the note about the 'ipv6' parameter is made obsolete 
in the actual implementation, which follows.
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 <osvob...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvob...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to