[PATCHES] services startup and identification

[Simo Sorce]

The following 3 patches changes the way services are started up and
identify themselves to the monitor.

The first patch is just minor or cosmetic fixes.

The second patch changes the identification from monitor initiated to
service initiated and acked by the monitor. This allows the service to
perform the identification after it has completely set up all the dbus
channel and is ready to receive a reply avoiding nasty race conditions.

The third patch changes the way services are started at startup. The
Data Provider service is given a 1 second advantage so that it should
normally be all set up by the time other service start and try to
connect to it. This way services normally avoid racing against dp and
can fully initialize on the first attempt and not rely on reconnection.

Initial testing seem to work pretty well and startup does not show ugly
errors anymore.

I may send an additional separate patch to make services identify
against dp the same way they do against the monitor (ie service
initiated instead of server initiated). A patch will follow if I can
pull it off in a short time frame.

Simo.

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/11/2009 06:52 AM, Simo Sorce wrote:
>  The following 3 patches changes the way services are started up and
>  identify themselves to the monitor.
>  
>  The first patch is just minor or cosmetic fixes.
>  
>  The second patch changes the identification from monitor initiated to
>  service initiated and acked by the monitor. This allows the service to
>  perform the identification after it has completely set up all the dbus
>  channel and is ready to receive a reply avoiding nasty race conditions.
>  
>  The third patch changes the way services are started at startup. The
>  Data Provider service is given a 1 second advantage so that it should
>  normally be all set up by the time other service start and try to
>  connect to it. This way services normally avoid racing against dp and
>  can fully initialize on the first attempt and not rely on reconnection.
>  
>  Initial testing seem to work pretty well and startup does not show ugly
>  errors anymore.
>  
>  I may send an additional separate patch to make services identify
>  against dp the same way they do against the monitor (ie service
>  initiated instead of server initiated). A patch will follow if I can
>  pull it off in a short time frame.
>  
>  Simo.
>  
>  
>  ------------------------------------------------------------------------
>  
>  _______________________________________________
>  sssd-devel mailing list
>  sssd-devel(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/sssd-devel 
Patch 0001: Ack
Patch 0002:
Minor typo in client_registration: regeistration should be registration
Coding standard for dbus methods suggest the more readable:
ret = dbus_message_append_args(reply,
                               DBUS_TYPE_STRING, &version,
                               DBUS_TYPE_INVALID);
Introduces a segmentation fault when an LDAP server is very slow. The
ping timeout is hit and frees the connection, but it subsequently
segfaults on a double-free.

I'd rather that we moved the monitor versions to integers. We never
actually used the dbus_get_monitor_version() call previously, so there's
no ABI break if we simply move the version to 1 now.

600000 seconds is way too long a timeout for monitor_common_send_id.
Prefer -1 (which libdbus defines as "a reasonable timeout") if you don't
know what to set.

Patch 0003: Ack.

- -- 
Stephen Gallagher
RHCE 804006346421761

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkqBZwQACgkQeiVVYja6o6OxSgCdENxAgzKu7hQbGwodqHMa7sr1
> nrAAnjAD2N77pgCMEUHPisV/m8+ABy9U
> =CWlq
> -----END PGP SIGNATURE-----

[Simo Sorce]

On Tue, 2009-08-11 at 08:41 -0400, Stephen Gallagher wrote:
>  Patch 0002:
>  Minor typo in client_registration: regeistration should be
>  registration 
ok

>  Coding standard for dbus methods suggest the more readable:
>  ret = dbus_message_append_args(reply,
>                                 DBUS_TYPE_STRING, &version,
>                                 DBUS_TYPE_INVALID); 
ok

>  Introduces a segmentation fault when an LDAP server is very slow.
> The
>  ping timeout is hit and frees the connection, but it subsequently
>  segfaults on a double-free. 
I added a spy to the connection structure so that the svc reference is
removed when it goes away, this should fix the segfault.

>  I'd rather that we moved the monitor versions to integers. We
> never
>  actually used the dbus_get_monitor_version() call previously, so
>  there's
>  no ABI break if we simply move the version to 1 now. 
the change in this patch is already a huge internal ABI and API break
anyway :-) But it doesn't matter until we reach v 1.0 and start seeing
people building external backends. Changed monitor version to be a hex
number as other ones.

>  600000 seconds is way too long a timeout for monitor_common_send_id.
>  Prefer -1 (which libdbus defines as "a reasonable timeout") if you
>  don't
>  know what to set. 
That's 600k milliseconds IIRC, not changing it as I simply reused what
was there before. We will have a session to review all timeouts and set
proper values once and for all.

Attached new 0002 patch with the included fixes.

Simo.

[Simo Sorce]

>  Attached new 0002 patch with the included fixes. 
Ooops patch was buggy, sent int tried to read string (thanks Steve :)

New one attached.

Simo.

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/11/2009 12:08 PM, Simo Sorce wrote:
> > Attached new 0002 patch with the included fixes.
>  
>  Ooops patch was buggy, sent int tried to read string (thanks Steve :)
>  
>  New one attached.
>  
>  Simo.
>  
>  
>  ------------------------------------------------------------------------
>  
>  _______________________________________________
>  sssd-devel mailing list
>  sssd-devel(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/sssd-devel 
Ack.

- -- 
Stephen Gallagher
RHCE 804006346421761

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkqBmcEACgkQeiVVYja6o6PN2wCdHBnjVKVqNAcPsKMu+rVOGpfJ
> XZIAn2GQlVpp2HQNlJZGCueQo5omt+8z
> =vuNu
> -----END PGP SIGNATURE-----

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/11/2009 12:18 PM, Stephen Gallagher wrote:
>  On 08/11/2009 12:08 PM, Simo Sorce wrote:
> >> Attached new 0002 patch with the included fixes.
> > Ooops patch was buggy, sent int tried to read string (thanks Steve :)
>  
> > New one attached.
>  
> > Simo.
>  
>  
> > ------------------------------------------------------------------------
>  
> > _______________________________________________
> > sssd-devel mailing list
> > sssd-devel(a)lists.fedorahosted.org
> > https://fedorahosted.org/mailman/listinfo/sssd-devel
>  
>  Ack.
>   
Pushed to master.
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

- -- 
Stephen Gallagher
RHCE 804006346421761

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkqBnJcACgkQeiVVYja6o6NupwCfYiypKHXsm8w5T7cYDUvPcmn1
> QvcAoJDUIK/oFToaxJmKRvhEzJNTySuw
> =9nTk
> -----END PGP SIGNATURE-----