Change in vdsm[master]: netinfo: Determine bootproto without ifcfg files

osvoboda at redhat.com osvoboda at redhat.com
Mon Jan 13 12:14:58 UTC 2014


Ondřej Svoboda has posted comments on this change.

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


Patch Set 2:

(6 comments)

http://gerrit.ovirt.org/#/c/23098/2/lib/vdsm/netinfo.py
File lib/vdsm/netinfo.py:

Next step(s): extend pgrep to find us both dhclient and dhcpcd. Use it in a function run from get(), passing it d['networks']. Walk the process list the same way as getBootProtocolDynamic() used to do and find out which interfaces from each of d['networks']'s nets are controlled by DHCP clients that are running.
Line 1: #
Line 2: # Copyright 2009-2013 Red Hat, Inc.
Line 3: #
Line 4: # This program is free software; you can redistribute it and/or modify


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 lev
I will remove this one and downgrade others.
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 for
Only assigning bootproto to networks, I assume it is enough to operate on d['networks'] in get().
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 comma
dhclient uses DHCPv6 iff there is -6 on the command line. dhcpcd uses both protocols at the same time by default with -4 and -6 forcing it to only use one of them. So to tell DHCPv4/6 apart seems dead simple, very unlike discovering the interface name...
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 getBootPr
getBootProtocolDynamic() was just a proof of concept. I want to use the dictionary of networks (see the last comment) in a standalone function. I will extract the networks' respective interfaces (from ['iface']) and walk the list of processes in one go, adding ['bootproto4'] and ['bootproto6'].
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;


Line 585:     #   to lease files
Line 586: 
Line 587:     for net, netAttr in networks().iteritems():
Line 588:         try:
Line 589:             d['networks'][net] = _getNetInfo(netAttr.get('iface', net),
How about passing d['networks'] (after it has been populated) to a dedicated function?
Line 590:                                              netAttr['bridged'], gateways,
Line 591:                                              ipv6routes,
Line 592:                                              netAttr.get('qosInbound'),
Line 593:                                              netAttr.get('qosOutbound'))


-- 
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 <osvoboda at redhat.com>
Gerrit-Reviewer: Assaf Muller <amuller at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list