On Mon, Sep 14, 2009 at 11:30:44AM -0400, Simo Sorce wrote: > On Mon, 2009-09-14 at 07:48 -0400, Stephen Gallagher wrote: > > Just a nitpick, but why did you replace sbus_conn_send_reply() in > > be_check_online with sbus_get_connection and dbus_connection_send()? > > They are functionally identical. (except that sbus_conn_send_reply() > > can > > get the connection in one fewer stack frame, since it can access the > > sbus_connection object directly) > > I merged together what previously was an async request. > It was only asking the id provider about offline status. > Since now the offline status is directly accessible by the > dp_provider_be.c there is no need to make a request to any provider, we > directly return the answer to dp. (this call is not used by anything > anyway so far but we were planning to use it to force a backend to go > offline so it will come handy later on). > > > Assuming I'm reading this correctly, we're talking about considering a > > single backend process as being online or offline as a whole. Why is > > this dependent only on the ID provider for the backend? > > It isn't, attached new patch that add be_mark_offline() calls also to > the auth backends. > > > Shouldn't we > > consider that if the authentication module or password change modules > > are offline that we are offline? > > yes, should be fixed in the new patch, now. > > > Furthermore, even if the ID provider is offline, if we have cached > > user > > information that allows us to initiate a connection to a still-live > > authentication provider, isn't that perfectly reasonable? > > No, also because this is just an initial patch to start building > infrastructure. If you remember we discussed the idea or allowing the > monitor to put a backend forcibly offline, in that case all providers > must respect this. Further more during auth we want to always refresh > users, so if the id part is not available we can as well auth from the > cached password (if caching passwords is allowed). > > This infrastructure is also need for DNS discovery later, where we want > a central place to tell a specific server is unreachable. > > In any case I would rather put the infrastructure in place now, and > tweak specific behaviors later. > > > I'd argue that if any ONE of the modules was the ultimate determinator > > of online/offline status, it should be authentication rather than > > identification. > > Nope the auth module contacts the servers more rarely than the id > backend (auths are rare compared to requests to get id information), so > normally the ID backend is more qualified. In any case I am not going to > make any provider king, all of them should be able to signal that > servers are unreachable. > > > Code itself is sensible, so this is a Nack until you can convince me > > that the approach itself is right. > > Let's see if the new patch and explanations are enough :) > > Simo. >
I agree, this patch is a good starting point and we can add fine tuning later. ACK. bye, Sumit _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
