https://fedorahosted.org/sssd/ticket/1110
[PATCH 1/4]
I've removed FIXME and move the DEBUG message to a better place.
I would have Acked that, but I agree with Jakub that this should be done as a
part of fix for #1125
[PATCH 2/4]
I've wrapped the data provider part with tevent request so I can reuse
the code in periodical updates.
Nack,
sdap_sudo.c:144 - please use sdap_handler_done() for better clarity
I'm not quuite sure why are you swicthing to EAGAIN in
sdap_sudo_load_sudoers_next_base(). I'd understand if you used EOK as well,
but this way I doubt it has any advantage (doesn't even add lucidity IMO).
[PATCH 3/4]
Adds two new functions in sudo sysdb api that can set/retrieve the value
of 'refreshed' attribute (equivalent to 'has_enumerated' attribute in
domain enumeration). If it is true, the first schedule will be delayed
for a few seconds so we don't slow down the start up process.
Nack,
I think that re-using sysdb_set_enumerated() and sysdb_has_enumarated() and
writing wrappers around them is far better than copying the code, which is
almost identical.
[PATCH 4/4]
Periodical update itself. It adds two new options:
ldap_sudo_refresh_enabled (bool) - equivalent to 'enumerate'
ldap_sudo_refresh_timeout (number) - equivalent to
'ldap_enumeration_refresh_timeout'
Nack,
Add new options to config description in src/config/, also please review if all
of your original SUDO-related options are there as well
I think that delaying the refresh for ldap_sudo_refresh_timeout should be used
instead of hardcoded 10 seconds. Or do you think that default value for the
option is too high for this?
Nitpick: in the definition of sdap_sudo_refresh_timer(), you miss the "static"
keyword
You are forgetting to free sudo_ctx in sdap_sudo_refresh_timer, which could be
potentially deadly on long-running systems, since the refresh_ctx isn't freed
anywhere as weel
Please use the standard _send/_retry/_done/_recv function naming in
sdap_sudo_timer.c
In sdap_sudo_refresh_send, you create request with struct
sdap_sudo_refresh_state as its state, but in the timeout handling procedure,
you retrieve callback data of that request as struct sdap_sudo_refresh_ctx.
Please correct that and keep in mind that since this is entirely different
struct assigned to the request, freeing the request won't free the sudo_ctx
created in sdap_sudo_refresh_timer().
Thanks
Jan