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

osvoboda at redhat.com osvoboda at redhat.com
Tue Jan 14 00:05:25 UTC 2014


Ondřej Svoboda has posted comments on this change.

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


Patch Set 2:

(4 comments)

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

> I do not submit to the benefits of having a single pgrep - certainly not on
Only now I found out that you commented. The hooking up is definitely not beautiful in that I should set networks' properties in one place. pgrep has been ready for a while.
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
> Good, let's be dhcp4-specific, to avoid false recognition.
Sadly, this can still happen for dhcpcd when it has 'ipv6only' in its config.
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?
> To me, having a dedicated helper function return a {iface: isdhcp} map to b
There is a kind of the chicken and the hen problem (we are talking about the get() function later on):
I need a list of interfaces first so I know what to look for in cmdlines of DHCP clients. But this list only complete after networks() have been queried and _getNetInfo called for each one of them.
So in the patch set 3 (which I pushed before noticing your comments) I assign the 'configured-using-dhcp' flag when networks and their interfaces are known, albeit not elegantly.
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),
> I find it less clear than adding
I can imagine having the information (iface using DHCP) cached first and assigned in the loop. But we need to read ifaces first which involves duplicating the loop with:
- netAttr.get('iface', net)
- and handling of KeyError -- wait, can it still occur? getIfaceCfg() deals with a missing ifcfg file already.
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: Antoni Segura Puimedon <asegurap 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