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 <osvob...@redhat.com> Gerrit-Reviewer: Assaf Muller <amul...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Ondřej Svoboda <osvob...@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