On Fri, 2011-06-17 at 12:58 +0200, Jirka Klimes wrote: > On Thursday 26 of May 2011 20:22:26 Dan Williams wrote: > > > > > > A patch doing that is in the attachment. > > > > A few concerns with the patch; it might get more complicated because of > > them. First, if the connection is deleted before the idle handler runs, > > we'll be left with garbage. So we'd need to do g_object_weak_ref() on > > the connection object, cache the idle ID, and run g_source_remove() on > > that idle ID from the weak ref callback. Next, in > > reset_connection_retries() we'd want to remove that weak ref when > > freeing the ResetRetriesData structure. > > > > Second, if the connection gets activated again manually by the user > > while we're waiting the 5 minutes then we should g_source_remove() the > > idle ID. That gets more complicated. > > > > Perhaps a simpler approach would be to have a single, global idle > > handler in the NMPolicy that runs over the connection list and resets > > the retries as appropriate? First, in device_state_changed() we could > > attach the current seconds-since-epoch time to the object using > > g_object_set_data() when its # retries reaches 0, and then if no reset > > idle handler was scheduled, schedule one for 300 seconds later. > > (otherwise allow the existing idle handler to run earlier since > > presumably it was scheduled by an earlier failed connection we want to > > reset). > > > > Then, when the reset idle handler does run, iterate over each > > connection, save the earliest reset timestamp, and reschedule the idle > > handler for that timestamp + 300 seconds. During this iteration of > > course we reset the retries count for every connection that has a reset > > timestamp earlier than now. > > > > If the connection gets activated, nm-policy.c's signal handlers can > > listen for that and clear out the invalid timestamp data too. > > > > If that would all work, that would allow us to avoid doing all the > > alloc/dealloc of a custom data structure, plus we only have to manage > > one idle handler. Plus we don't have to care too much about stuff like > > connection deletion and activation happening before our idle handler > > runs since those will be easier to deal with. > > > > Thoughts? > > > > Dan > > Please find the reworked patch is the attachment.
Looks good, though for completeness we'd want to g_source_remove() the timeout's ID in nm_policy_destroy() too. It won't really have an effect since nm_policy_destroy() is only called when NM is exiting though. Thanks! Dan _______________________________________________ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list