On Mon, 2011-06-20 at 15:42 -0400, Stephen Gallagher wrote:
Ok, I'm going to try to summarize the responses from Simo and
Jakub,
then hopefully we'll accept them and we can add this information to the
coding and contribution guidelines. (I have intentionally broken the
thread).
I agree with Simo that we should break this into a bitfield. I'm going
to suggest that we consider a match of 0x000F to be an error, 0x00F0 to
be informational and 0x0F00 to be developer. This adjusts Simo's numbers
slightly, but it makes a little more room available at each level in
case we want to add a new level in the middle.
I also think we should change the config file option name to "debug"
rather than "debug_mask". I think it's more readable.
#define SSSDBG_FATAL_FAILURE 0x0000 /* level 0 */
Fatal failures. Anything that would prevent SSSD from starting up or
causes it to cease running. (Example: sssd.conf fails to parse)
#define SSSDBG_CRIT_FAILURE 0x0001 /* level 1 */
Critical failures. An error that doesn't kill the SSSD, but one that
indicates that at least one major feature is not going to work properly
(Example: Kerberos keytab does not have a valid principal for
SASL/GSSAPI LDAP)
#define SSSDBG_OP_FAILURE 0x0002 /* level 2 */
Serious failures. An error announcing that a particular request or
operation has failed. (Example: DP returns PAM_SYSTEM_ERR)
#define SSSDBG_MINOR_FAILURE 0x0004 /* level 3 */
Minor failures. These are the errors that would percolate down to
cause the operation failure of 2. (Example: The above PAM_SYSTEM_ERR was
due to the krb5_child returning an unexpected error)
#define SSSDBG_CONF_SETTINGS 0x0010 /* level 4 */
Configuration settings (E.g. the dp_option loading)
#define SSSDBG_FUNC_DATA 0x0020 /* level 5 */
Function data (E.g. LDAP search request data, Dynamic DNS message)
#define SSSDBG_TRACE_FUNC 0x0040 /* level 6 */
Trace messages for operation functions (e.g. getAccountInfo,
pamHandler)
#define SSSDBG_TRACE_LIBS 0x0100 /* level 7 */
Trace messages for internal control functions (e.g. ping,
resetOffline)
#define SSSDBG_TRACE_INTERNAL 0x0200 /* level 8 */
Contents of function-internal variables that may be interesting
#define SSSDBG_TRACE_ALL 0x0400 /* level 9 */
Extremely low-level tracing information (Example: adding/removing
D-BUS watches)
For backwards compatibility, we would map the existing debug_level
values as below:
SSS_DEBUG0: SSSDBG_FATAL_FAILURE
SSS_DEBUG1: SSS_DEBUG0 & SSSDBG_CRIT_FAILURE
SSS_DEBUG2: SSS_DEBUG1 & SSSDBG_OP_FAILURE
SSS_DEBUG3: SSS_DEBUG2 & SSSDBG_MINOR_FAILURE
SSS_DEBUG4: SSS_DEBUG3 & SSSDBG_CONF_SETTINGS
SSS_DEBUG5: SSS_DEBUG4 & SSSDBG_FUNC_DATA
SSS_DEBUG6: SSS_DEBUG5 & SSSDBG_TRACE_FUNC
SSS_DEBUG7: SSS_DEBUG6 & SSSDBG_TRACE_LIBS
SSS_DEBUG8: SSS_DEBUG7 & SSSDBG_TRACE_INTERNAL
SSS_DEBUG9: SSS_DEBUG8 & SSSDBG_TRACE_ALL
I guess you meant to use | on each line above here, not & ?
If we agree on these definitions, I will write it up on the wiki and
we'll put the following policy in place:
All new code MUST use the new DEBUG definitions. Existing code being
modified SHOULD convert related DEBUG messages to the new macros. It is
NOT required to mass-convert existing messages at this time, but patches
are welcome :)
By "new DEBUG definitions" I hope you mean the SSSDBG_* ones while the
SSS_DEBUGN are considered legacy ?
Also the problem with this scheme is that you'll have to change every
single debug statement in the code at once.
If we move all 4 bits upper we can treat any number < 0x0F as a legacy
code. So we would not need to define SSS_DEBUGN macros, plus we can
avoid changing DEBUG macros all at once, but change them only when we
change code around them.
This would mean:
#define SSSDBG_MASK_LEGACY 0x000F
#define SSSDBG_MASK_FATAL 0x00F0
#define SSSDBG_MASK_INFO 0x0F00
#define SSSDBG_MASK_DEBUG 0xF000
(Adjust list appropriately)
Simo.
--
Simo Sorce * Red Hat, Inc * New York