GNOME Bugzilla – Bug 723350
NM 0.9.8.8 doesn't properly set the default connectivity check interval if it's not set in config
Last modified: 2015-05-19 14:45:12 UTC
Created attachment 267710 [details] [review] set default value for connectivity check interval if missing While the man page mentions that the interval would be set to 300 seconds when missing from /etc/NetworkManager/NetworkManager.conf, it appears to remain at 0. This breaks the first check for connectivity on connection; the initial run_check never run, and not timeout is set for a recurring check.
Review of attachment 267710 [details] [review]: ::: src/nm-config.c @@ +196,3 @@ + */ + config->connectivity_interval = NM_CONNECTIVITY_DEFAULT_INTERVAL; + } Need to g_clear_error (&err); here. ::: include/NetworkManager.h @@ +70,3 @@ +#define NM_CONNECTIVITY_DEFAULT_INTERVAL 300 + I'd rather keep this out of NetworkManager.h since it's not really useful as part of the API. Instead, I'd like to put it into nm-config.h I think.
Created attachment 296337 [details] nm-connectivity: default the check interval to 300 seconds Hm, this no longer applies and I find leetting the NMConfig know about the NMConnectivity defaults somewhat unnecessarily complex. However the bug is still there. Would this work instead?
Created attachment 297264 [details] [review] connectivty: allow missing connectivity.interval argument as fallback to 300 Manual page claims that a missing configuration option for connectivity interval means "300". That was not the case for a long time (never?).
if this behavior was never implemented as the manual page claims, we could also fix the manual page and don't change the source. Otherwise, I agree to the patch from comment 1 -- apart from NM_CONNECTIVITY_DEFAULT_INTERVAL. I think the right place for it is in nm-config.h On master, the correct solution is IMO comment 3, not comment 2 :)
(In reply to Lubomir Rintel from comment #2) > Created attachment 296337 [details] > nm-connectivity: default the check interval to 300 seconds > > Hm, this no longer applies and I find leetting the NMConfig know about the > NMConnectivity defaults somewhat unnecessarily complex. IMO it's correct that NMConfig is aware of the default values for configurations...
Pushed a modified version of attachment 297264 [details] [review] to master: http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=652853e0d0749a12db07e34f1a3cd901020a76f8 nm-1-0: http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=928494af2bde1c188943b126c018a9247cd849b1 As I said, I am pretty sure that attachment 296337 [details] is wrong, because it coerces 0 to 300 -- but zero is valid and means "disabled".
I also applied a modified version of attachement 267710 on nm-0-9-8: http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=24e9b904279ddf6107ffde2609db5565cda5ffb5 and backported from master to nm-0-9-10: http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=c12d6edf23c4ae262e5addf1a891203167717511