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

Michal Židek mzidek at redhat.com
Thu Aug 13 13:49:27 UTC 2015


On 08/13/2015 03:22 PM, Pavel Březina wrote:
> 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?
>

Will do. I was thinking if I should do it with this patch or not.
But because we have one more level unused I decided to postpone
it. But I will do it.

Michal



More information about the sssd-devel mailing list