Hi Patrik,

On Fri, Jan 04, 2013 at 12:33:36PM +0200, Patrik Flykt wrote:
> On Wed, 2012-12-05 at 16:44 -0500, Forest Bond wrote:
> > From: Forest Bond <[email protected]>
> > 
> > This is needed to make a service go online in the case where it was
> > already connected and then manual IPv4 & nameservers settings are
> > applied.  In that case, wispr is restarted with the new IP settings, but
> > the nameservers have not been set yet, so the wispr test fails and the
> > service remains in ready state.
> > ---
> >  src/service.c |   10 ++++++++++
> >  1 files changed, 10 insertions(+), 0 deletions(-)
> > 
> > diff --git a/src/service.c b/src/service.c
> > index 67889de..88470ad 100644
> > --- a/src/service.c
> > +++ b/src/service.c
> > @@ -3106,6 +3106,16 @@ static DBusMessage *set_property(DBusConnection 
> > *conn,
> >             update_nameservers(service);
> >             dns_configuration_changed(service);
> >  
> > +           if (__connman_service_is_connected_state(service,
> > +                                           CONNMAN_IPCONFIG_TYPE_IPV4))
> > +                   __connman_wispr_start(service,
> > +                                           CONNMAN_IPCONFIG_TYPE_IPV4);
> > +
> > +           if (__connman_service_is_connected_state(service,
> > +                                           CONNMAN_IPCONFIG_TYPE_IPV6))
> > +                   __connman_wispr_start(service,
> > +                                           CONNMAN_IPCONFIG_TYPE_IPV6);
> > +
> >             service_save(service);
> >     } else if (g_str_equal(name, "Timeservers.Configuration") == TRUE) {
> >             DBusMessageIter entry;
> 
> This is all good, but in order to take every nameserver update into
> account I think wispr check should be done in/around
> update_nameservers() if the service is in state ready and nameservers
> exist. The trouble here would be to figure out when
> __connman_service_nameserver_remove() is being used and not start the
> wispr check at that point.
> 
> But maybe that is too much work and we'll go with the proposed solution
> anyway. I'll need to think about this some more. Any comments anyone?

My patch fixes the initial wispr check on service reconfiguration, which is
fragile and depends on the service properties being set in a particular order.
This prevents a service from properly entering online state.  It's a bug.

The expanded checks you refer to would solve a slightly different problem
(taking a service *out* of online state when the service configuration has
changed).  It's an enhancement.

I think the patch should be applied as-is.  Then we can follow up with the
expanded wispr checks if we want them.  (Although I'll admit that I'm skeptical
they have much value unless we also periodically recheck connectivity to catch
e.g. upstream routing problems).

Thanks,
Forest
-- 
Forest Bond
http://www.alittletooquiet.net
http://www.rapidrollout.com

Attachment: signature.asc
Description: Digital signature

_______________________________________________
connman mailing list
[email protected]
http://lists.connman.net/listinfo/connman

Reply via email to