GNOME Bugzilla – Bug 708875
Don't warn about ignoring non-existent slave IP config
Last modified: 2013-10-14 16:16:06 UTC
ifcfg-rh *always* warns that it's ignore a slave's IP config, even when there is no IP config. first patch is a drive-by code simplification also available at danw/ipwarning
Created attachment 255883 [details] [review] ifcfg-rh: fix handling of minimal ifcfg files ifcfg-rh had the rule that if an ifcfg file had no BOOTPROTO and no IPv4 addresses, then it should be treated as BOOTPROTO=dhcp for compatibility. But in fact, current ifup treats it as BOOTPROTO=none, so we should too.
Created attachment 255884 [details] [review] ifcfg-rh: don't warn about ignoring non-existent slave IP config connection_from_file() warns if a slave connection has any IP4 or IP6 config, but make_ip4_setting() and make_ip6_setting() would always return a setting (possibly with method=DISABLED/IGNORE) unless there was a parse error. So this meant the warning would get printed even if the file had no IP config. Fix that.
Created attachment 255936 [details] [review] settings: make connections always have s_ip4 and s_ip6 Make sure that all connections returned from NMSettings have an NMSettingIP4Config and an NMSettingIP6Config (so we don't need to special-case NULL elsewhere), and move the slaves-can't-have-IP-config checks into the platform-independent code as well. This also gets rid of spurious "ignoring IP4/IP6 configuration" warnings in ifcfg-rh when reading a slave ifcfg file. Partly based on a patch from Pavel.
Created attachment 255937 [details] [review] ifcfg-rh: fix handling of minimal ifcfg files ifcfg-rh had the rule that if an ifcfg file had no BOOTPROTO and no IPv4 addresses, then it should be treated as method=auto for compatibility. But in fact, current ifup treats it as method=disabled, so we should too.
Review of attachment 255936 [details] [review]: Looks good except NetworkManagerUtils.c could be removed from the patch (whitespace-only change).
(In reply to comment #5) > Looks good except NetworkManagerUtils.c could be removed from the patch > (whitespace-only change). oops, yeah, that was leftover from an earlier version of the patch where the normalize function was in that file instead of nm-settings-utils.c
Comment on attachment 255937 [details] [review] ifcfg-rh: fix handling of minimal ifcfg files the branch now has newer code than this
Thomas reviewed this as part of bug 700414
Jiri pointed out that always having IP4 and IP6 configs made some other things weird. Eg, nmcli would show the full NMSettingIP4Config and NMSettingIP6Config, and connections saved by the keyfile plugin might also show some ip4/ip6 properties set that would not be meaningful. I've pushed a new danw/ipwarning that updates things so that slaves are now guaranteed to not have ip configs, and everything else is guaranteed to have them.
The patch looks good to me.
fixed