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 <[email protected]> Gerrit-Reviewer: Assaf Muller <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Ondřej Svoboda <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
