URL: https://github.com/SSSD/sssd/pull/94
Title: #94: Enable {socket,dbus}-activation for responders
pbrezina commented:
"""
**Unit files**
I wonder if the user and group in unit files should take --with-sssd-user value
from configure.
**MAN: Mention that the services' list is optional**
It should also say that it is socket activated if it is missing.
**sbus timeout**
```c
struct sbus_service_timeout {
errno_t (*setup_timeout_fn)(void *);
errno_t (*reset_timeout_fn)(void *);
void *timeout_data;
};
```
`setup_timeout_fn` is not used in sbus, it is used in monitor code so it should
not be part of sbus. I agree to setup the idle timeout in monitor when we know
that the service was bus activated. But I still don't think we need to expose
this logic in sbus. Pass the function pointer to the monitor (although I think
it can be hardcoded) and only pointer to time_t of last request time to sbus.
Also I would prefer to delete `reset_responder_idle_timer` and only set
rctx->last_request_time directly without all those checks and a debug message.
I believe it will be more simple and those checks are not really necessary (it
doesn't matter if the timer is set or not, we still can remember last request
time).
**sbus_destructor**
Destructors with talloc can be typed directly and still keep type checking with
gcc:
```c
static int sbus_connection_destructor(void *ctx)
-> static int sbus_connection_destructor(struct sbus_connection *conn)
```
**ensure that timeout is at least 60 seconds**
```c
+ /* Idle timeout set to 0 means that no timeout will be set up to
+ * the responder */
+ if (rctx->idle_timeout != 0) {
+ /* Ensure that the responder timeout is at least sixty seconds */
+ if (rctx->idle_timeout < 60) {
+ rctx->idle_timeout = 60;
+ }
}
```
Print a warning here, please.
**sudo**
Sudo is a little bit special responder. Since it requires some periodic task in
the backend, we decided a long time ago to disable sudo in backend unless it is
explicitly configured, either by putting "services = sudo" or "sudo_provider =
provider". I think this is a good time to treat it the same as all other
responders, i.e. keep it always enabled unless it is explicitly disabled with
`sudo_provider = none`.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/94#issuecomment-266420800
_______________________________________________
sssd-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
