Dan Kenigsberg has posted comments on this change.

Change subject: netinfo: Determine bootproto without ifcfg files
......................................................................


Patch Set 2:

(4 comments)

....................................................
File lib/vdsm/netinfo.py
Line 447:         raise NotImplementedError
Line 448: 
Line 449: 
Line 450: def getBootProtocolDynamic(iface):
Line 451:     logging.info('Looking for DHCP client for %s.', iface)
I see no reason to include so many logging lines, certainly not in INFO level.
Line 452: 
Line 453:     for pid in pgrep('dhclient'):
Line 454:         if iface in getCmdArgs(pid):
Line 455:             logging.info('Found dhclient running for %s.', iface)


Line 465:     # There are a bunch of "network managers" around but all of them
Line 466:     #   hopefully only use either dhclient or dhcpcd
Line 467:     #   => not worth asking them or is NetworkManager so widespread?
Line 468:     #
Line 469:     # Walk the process list only once for all interfaces
Right. It should not be hard to define a helper function that returns a 
forzenset() of all interfaces with dhcp.
Line 470:     #
Line 471:     # Treat IPv4 and IPv6 independently
Line 472:     #
Line 473:     # Fall back to getBootProtocol?


Line 467:     #   => not worth asking them or is NetworkManager so widespread?
Line 468:     #
Line 469:     # Walk the process list only once for all interfaces
Line 470:     #
Line 471:     # Treat IPv4 and IPv6 independently
that's important indeed. Can we easily discern than from the dhclient command 
line? Not sure at all. However, since IPv6 is still very experimental, you may 
leave bootproto6 under a big TODO sign.
Line 472:     #
Line 473:     # Fall back to getBootProtocol?
Line 474:     #
Line 475:     # Try harder (how much?) in cases a device name is not explicitly 
specified


Line 469:     # Walk the process list only once for all interfaces
Line 470:     #
Line 471:     # Treat IPv4 and IPv6 independently
Line 472:     #
Line 473:     # Fall back to getBootProtocol?
at first stage, please fall back to the current implementation of 
getBootProtocol(), and that's that. Note that there is no need for a specific 
getBootProtocolDynamic() function - we simply improve getBootProtocol() so it 
works without ifcfg*.
Line 474:     #
Line 475:     # Try harder (how much?) in cases a device name is not explicitly 
specified
Line 476:     #   on command line / or DHCP client already gone but its lease 
still valid
Line 477:     # This possibly includes: defaulting to all interfaces; dhcpcd's 
globbing;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5fbc48a0adf5f40120a72ec2c4cc2fc80b7226b8
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvob...@redhat.com>
Gerrit-Reviewer: Assaf Muller <amul...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
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