[SSSD] [PATCH] CONFDB: Assume config file version 2 if missing
Michal Židek
mzidek at redhat.com
Tue Sep 1 10:55:29 UTC 2015
On 09/01/2015 11:11 AM, Jakub Hrozek wrote:
> On Tue, Sep 01, 2015 at 11:00:15AM +0200, Lukas Slebodnik wrote:
>> On (01/09/15 10:51), Jakub Hrozek wrote:
>>> On Tue, Aug 11, 2015 at 07:47:00AM +0200, Lukas Slebodnik wrote:
>>>> On (10/08/15 17:18), Michal Židek wrote:
>>>>> On 08/06/2015 01:39 PM, Lukas Slebodnik wrote:
>>>>>> On (05/08/15 13:41), Michal Židek wrote:
>>>>>>> On 07/30/2015 06:08 PM, Lukas Slebodnik wrote:
>>>>>>>>>>> - } else if (version < CONFDB_VERSION_INT) {
>>>>>>>>>>> - DEBUG(SSSDBG_FATAL_FAILURE,
>>>>>>>>>>> - "Config file is an old version. "
>>>>>>>>>>> - "Please run configuration upgrade script.\n");
>>>>>>>>>>> - ret = EINVAL;
>>>>>>>>>>> - goto done;
>>>>>>>>>>> - } else if (version > CONFDB_VERSION_INT) {
>>>>>>>>>>> - DEBUG(SSSDBG_FATAL_FAILURE,
>>>>>>>>>>> - "Config file version is newer than confdb\n");
>>>>>>>>>>> - ret = EINVAL;
>>>>>>>>>>> - goto done;
>>>>>>>>>>> + /* No known version. Use default. */
>>>>>>>>>>> + DEBUG(SSSDBG_CONF_SETTINGS,
>>>>>>>>>>> + "Value of config_file_version option not found. "
>>>>>>>>>>> + "Assumed to be version %d.\n", CONFDB_DEFAULT_CFG_FILE_VER);
>>>>>>>>>>> + } else {
>>>>>>>>>>> + version = sss_ini_get_int_config_value(init_data,
>>>>>>>>>>> + CONFDB_DEFAULT_CFG_FILE_VER,
>>>>>>>>>>> + -1, &ret);
>>>>>>>>>>> + if (ret != EOK) {
>>>>>>>>>>> + DEBUG(SSSDBG_FATAL_FAILURE,
>>>>>>>>>>> + "Config file version could not be determined\n");
>>>>>>>> ^^
>>>>>>>> I do not prefer nested "if"s. If you decided to do it in this way then
>>>>>>>
>>>>>>> Me neither, but sss_ini_get_int_config_value() has to be
>>>>>>> skipped conditionally. It is just call to the function
>>>>>>> plus error checking that is nested. I think it is not
>>>>>>> too bad in this case.
>>>>>>>
>>>>>>>> you shoudl have proper indentation.
>>>>>>>>
>>>>>>>
>>>>>>> Fixed in the new version.
>>>>>>>
>>>>>> It works because integration tests passed.
>>>>>> http://sssd-ci.duckdns.org/logs/job/20/68/summary.html
>>>>>> But ...
>>>>>>
>>>>>> I tested new version with ipa-client-install
>>>>>> and "config_file_version = 2" is still added to sssd.conf
>>>>>> even though it is a default value.
>>>>>>
>>>>>> ipa-client-install uses our python API (python-sssdconfig)
>>>>>> and it does not try to add this option itself.
>>>>>
>>>>> I often change version of SSSD with git checkout sssd<version>.
>>>>> If I generate the config with realmd or ipa-client-install
>>>> realmd do not use python module SSSDConfig.
>>>>
>>>>> with latest version then the config_version_file
>>>>> would need to be added manually (and I am pretty
>>>>> sure it would be after I looked into logs to see why sssd
>>>>> is not starting). I know this will probably only hit
>>>>> testers/developers, but I would prefer not to add unnecessary
>>>> just developers and developers useually join machine
>>>> to AD or IPA just once.
>>>>
>>>>> little inconveniences.
>>>>>
>>>> I do not try old sssd version very often. Just in case of bisect.
>>>> and ther is nice/simple workarount for "little inconvenience"
>>>>
>>>> Just manually add "config_version_file = 2" to sssd.conf
>>>>
>>>> The main point of this ticket is to simplify sssd.conf
>>>> and our python sssdconfig API should do the same.
>>>> Otherwise we do not need to do such change.
>>>>
>>>> I still see "config_file_version = 2" in sssd.conf
>>>> It was generated by python module SSSDConfig (via ipa-client-install)
>>>>
>>>> LS
>>>
>>> This thread has stalled, let's try to restart it.
>>>
>>> What versions are normally still being worked on by developers? I
>>> suspect it's master and sssd-1-12. What about pushing the patch to
>>> sssd-1-12 as well?
>> I'm fine with sssd-1-12.
>> The python sssdconfig need to be updated.
>>
>> LS
>
> Michal, if you agree, please update the patches so that we can push them
> in time for sssd-1.13.1
New patch attached. I tested ipa-client-install and the
config_file_version is no longer added.
Michal
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-CONFDB-Assume-config-file-version-2-if-missing.patch
Type: text/x-patch
Size: 7845 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150901/71591eaa/attachment-0001.bin>
More information about the sssd-devel
mailing list