Change in vdsm[master]: netinfo: Determine bootproto without ifcfg files
danken at redhat.com
danken at redhat.com
Mon Jan 13 21:15:29 UTC 2014
Dan Kenigsberg has posted comments on this change.
Change subject: netinfo: Determine bootproto without ifcfg files
......................................................................
Patch Set 2:
(4 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
I do not submit to the benefits of having a single pgrep - certainly not on the first round.
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 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
> dhclient uses DHCPv6 iff there is -6 on the command line. dhcpcd uses both
Good, let's be dhcp4-specific, to avoid false recognition.
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?
> getBootProtocolDynamic() was just a proof of concept. I want to use the dic
To me, having a dedicated helper function return a {iface: isdhcp} map to be used by get() is a bit clearer, and has the same complexity.
I you feel otherwise - no problem, just show the code so it can be judged.
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 dedicate
I find it less clear than adding
dhcpmap = _getDhcpPerIface()
and setting
d['networks'][net]['bootproto'] = dhcpmap(netAttr.get('iface', net))
does this make sense to you?
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