[SSSD] [PATCH] Display more information on DB version crash

Ondrej Kos okos at redhat.com
Tue Nov 13 14:41:34 UTC 2012


On 11/12/2012 01:14 PM, Jan Cholasta wrote:
> On 9.11.2012 13:24, Ondrej Kos wrote:
>> On 11/08/2012 07:01 PM, Jan Cholasta wrote:
>>> Hi,
>>>
>>> On 8.11.2012 15:05, Ondrej Kos wrote:
>>>> https://fedorahosted.org/sssd/ticket/1589
>>>>
>>>> patch is attached
>>>>
>>>> O.
>>>>
>>>
>>> 1) I think monitor and responders should use a different error message,
>>> excluding "If greater version is expected, run SSSD ...". Telling users
>>> to run SSSD when they in fact have it running might be confusing for
>>> someone.
>>>
>>> It might be even better to have different error messages for when the
>>> version is greater than expected and when the version is lesser than
>>> expected (not sure how hard it would be to implement that).
>>>
>>> 2) You should check for EMEDIUMTYPE in sss_cache.c and tools_util.c
>>> right after sysdb_init_domain_and_sysdb is called IMO.
>>>
>>> 3) I would prefer if you did not suppress logging of debug messages when
>>> ret == EMEDIUMTYPE.
>>>
>>> Honza
>>>
>>
>> 1 - distinguished whether reporting from daemon or tool, added another
>> errno return value - EUCLEAN meaning database version is higher then
>> expected
>>
>> 2 - fixed
>>
>> 3 - fixed
>>
>> new patch attached
>>
>> O.
>>
>
> +errno_t
> +sysdb_version_diff(const char *expected,
> +                           const char *received)
> +{
> +    return (atof(expected) < atof(received)) ? EUCLEAN : EMEDIUMTYPE;
> +}
> +
>
> I don't think you can use atof here, as it is locale-dependent. For
> example, when the current locale is cs_CZ, then atof("0.13") == 0.0,
> because in Czech the decimal mark is ",", so atof will stop parsing when
> it hits the ".".
>
> I think it would be nice (though not necessary) to add a generic
> function for comparing version strings to util and use it for the
> comparison.
>
>
> You use this:
>
> +        if (ret == EMEDIUMTYPE || ret == EUCLEAN) {
> +            sss_db_version_mismatch(ret, true);
> +        }
>
> everywhere in the patch, I think you should make a macro out of it.
>
>
> Please add src/util/util.c to po/POTFILES.in, so that gettext can pick
> up the error messages you have added for translation.
>
>
> Honza
>

New patch attached (without adding src/util/util.c to po/POTFILES.in 
since there's new bug filed for it)

O.


-- 
Ondrej Kos
Associate Software Engineer
Identity Management
Red Hat Czech

phone: +420-532-294-558
cell:  +420-736-417-909
ext:   82-62558
loc:   1013 Brno 1 office
irc:   okos @ #brno
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Display-more-information-on-DB-version-crash.patch
Type: text/x-patch
Size: 8217 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20121113/ed5e8cc6/attachment.bin>


More information about the sssd-devel mailing list