[SSSD] [PATCH] DEBUG: Add new debug category for fail over

Pavel Březina pbrezina at redhat.com
Thu Aug 13 13:22:54 UTC 2015


On 08/12/2015 07:09 PM, Michal Židek wrote:
> On 08/12/2015 06:24 AM, Lukas Slebodnik wrote:
>> On (11/08/15 15:00), Michal Židek wrote:
>>> On 08/11/2015 01:25 PM, Lukas Slebodnik wrote:
>>>> On (11/08/15 13:16), Michal Židek wrote:
>>>>> On 08/11/2015 06:52 AM, Lukas Slebodnik wrote:
>>>>>> On (10/08/15 19:28), Michal Židek wrote:
>>>>>>> See attached patch that adds debug messages
>>>>>>>
>>>>>>>
>>>>>>> Michal
>>>>>>
>>>>>
>>>>>
>>>>> Sorry that version of the patch was wrong.
>>>>>
>>>>> The attached is a proper version.
>>>>>
>>>>>> >From 8d3a54ee023ccb5bc37201a7c2fb1ec671134c1f Mon Sep 17 00:00:00
>>>>>> 2001
>>>>>>> From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzidek at redhat.com>
>>>>>>> Date: Mon, 10 Aug 2015 18:35:16 +0200
>>>>>>> Subject: [PATCH] DEBUG: Add new debug category for fail over.
>>>>>>>
>>>>>>> ---
>>>>>>> src/providers/data_provider_fo.c |  8 ++++----
>>>>>>> src/providers/dp_backend.h       | 27 +++++++++++++++++++++++----
>>>>>>> src/util/util.h                  |  1 +
>>>>>>> 3 files changed, 28 insertions(+), 8 deletions(-)
>>>>>>>
>>>>>>> diff --git a/src/providers/data_provider_fo.c
>>>>>>> b/src/providers/data_provider_fo.c
>>>>>>> index dab796d..de8a223 100644
>>>>>>> --- a/src/providers/data_provider_fo.c
>>>>>>> +++ b/src/providers/data_provider_fo.c
>>>>>>> @@ -743,10 +743,10 @@ void reset_fo(struct be_ctx *be_ctx)
>>>>>>>      fo_reset_services(be_ctx->be_fo->fo_ctx);
>>>>>>> }
>>>>>>>
>>>>>>> -void be_fo_set_port_status(struct be_ctx *ctx,
>>>>>>> -                           const char *service_name,
>>>>>>> -                           struct fo_server *server,
>>>>>>> -                           enum port_status status)
>>>>>>> +void _be_fo_set_port_status(struct be_ctx *ctx,
>>>>>>> +                            const char *service_name,
>>>>>>> +                            struct fo_server *server,
>>>>>>> +                            enum port_status status)
>>>>>>> {
>>>>>>>      struct be_svc_data *be_svc;
>>>>>>>
>>>>>>> diff --git a/src/providers/dp_backend.h b/src/providers/dp_backend.h
>>>>>>> index e4e22ea..18a78a0 100644
>>>>>>> --- a/src/providers/dp_backend.h
>>>>>>> +++ b/src/providers/dp_backend.h
>>>>>>> @@ -259,10 +259,29 @@ struct tevent_req
>>>>>>> *be_resolve_server_send(TALLOC_CTX *memctx,
>>>>>>>                                            bool first_try);
>>>>>>> int be_resolve_server_recv(struct tevent_req *req, struct
>>>>>>> fo_server **srv);
>>>>>>>
>>>>>>> -void be_fo_set_port_status(struct be_ctx *ctx,
>>>>>>> -                           const char *service_name,
>>>>>>> -                           struct fo_server *server,
>>>>>>> -                           enum port_status status);
>>>>>>> +#define be_fo_set_port_status(ctx, \
>>>>>> You can change be_fo_set_port_status to new static function with
>>>>>> prefix "_"
>>>>>> and backwark compatible function wit the same name will be thin
>>>>>> wrapper
>>>>>> around it. We should not abuse macro.
>>>>>>
>>>>>> If you are woried about performance it's better to reply on gcc
>>>>>> and inlining of
>>>>>> functions. It's better to rely on type check of functions.
>>>>>>
>>>>>
>>>>> No. The puprpose of the patch (what you could not see from
>>>>> the old version) was to print the line and file from which the
>>>>> function was called. This can only be done using the macro.
>>>>>
>>>> It can be achieved even without complicated do while macro.
>>>>
>>>> #define be_fo_set_port_status(ctx, service_name, server, status) \
>>>>      _be_fo_set_port_status(ctx, service_name, server, status, \
>>>>                             __FILE__, __LINE__)
>>>>
>>>> and log messages will be added to _be_fo_set_port_status.
>>>>
>>>> The macro will much simpler and any compilation errors/typos
>>>> can be easily found in the function _be_fo_set_port_status.
>>>>
>>>> @more inspiration -> talloc.h or tevent.h
>>>>
>>>> LS
>>>
>>> I agree that the code would look better, but would it
>>> not make the debug message a little too long
>> We do not have a limit for line length in log file.
>> Some messages are very long; especially if they contains DN or LDAP
>> filter.
>>
>
> True. And it indeed looks much better this way.
>
> New patch attached.
>
> Michal

Kudos for adding new level. Can you also create a patch, that would 
change the debug level constants to 0x00000000 format so it is clear 
that it is 32bits and there is still lots of space to add new levels?





More information about the sssd-devel mailing list