On (27/07/15 16:12), Michal Židek wrote:
> On 07/27/2015 04:05 PM, Lukas Slebodnik wrote:
>> On (27/07/15 15:58), Michal Židek wrote:
>>> On 07/24/2015 09:51 PM, Lukas Slebodnik wrote:
>>>> On (07/07/15 19:45), Michal Židek wrote:
>>>>> Hi!
>>>>>
>>>>> See the attached patch.
>>>>>
>>>>> Ticket:
>>>>>
https://fedorahosted.org/sssd/ticket/2688
>>>>>
>>>>> CI passed:
>>>>>
http://sssd-ci.duckdns.org/logs/job/18/71/summary.html
>>>>>
>>>>> Thanks,
>>>>> Michal
>>>>>
>>>>> --
>>>>> Senior Principal Intern
>>>>
>>>> >From c67f8df99df4f1aafa41d57a84c8d1c19fb5d610 Mon Sep 17 00:00:00
2001
>>>>> From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzidek(a)redhat.com>
>>>>> Date: Tue, 7 Jul 2015 15:15:32 +0200
>>>>> Subject: [PATCH] CONFDB: Assume config file version 2 if missing
>>>>>
>>>>> Default to config file version 2 if the version
>>>>> is not specified explicitly.
>>>>>
>>>>> Ticket:
>>>>>
https://fedorahosted.org/sssd/ticket/2688
>>>>> ---
>>>>> src/confdb/confdb.h | 1 +
>>>>> src/confdb/confdb_setup.c | 48
++++++++++++++--------------
>>>>> src/config/SSSDConfig/sssd_upgrade_config.py | 3 +-
>>>>> 3 files changed, 27 insertions(+), 25 deletions(-)
>>>>>
>>>> I think you can also remove config_file_version from
>>>> sssd.conf in integration test. Or at least comment it out.
>>>> So it will be proven that your patch works.
>>>>
>>>> LS
>>>
>>> Ok, I added patch that removes the option from
>>> sssd.conf in integration tests.
>>>
>>> Michal
>>>
>>> --
>>> Senior Principal Intern
>>
>> >From af960ad090a06017ce3880abaee9c07bbfbd8111 Mon Sep 17 00:00:00 2001
>>> From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzidek(a)redhat.com>
>>> Date: Tue, 7 Jul 2015 15:15:32 +0200
>>> Subject: [PATCH 1/2] CONFDB: Assume config file version 2 if missing
>>>
>>> Default to config file version 2 if the version
>>> is not specified explicitly.
>>>
>>> Ticket:
>>>
https://fedorahosted.org/sssd/ticket/2688
>>> ---
>>> src/confdb/confdb.h | 1 +
>>> src/confdb/confdb_setup.c | 48
++++++++++++++--------------
>>> src/config/SSSDConfig/sssd_upgrade_config.py | 3 +-
>>> 3 files changed, 27 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
>>> index 36df6ae..d2d7ebf 100644
>>> --- a/src/confdb/confdb.h
>>> +++ b/src/confdb/confdb.h
>>> @@ -38,6 +38,7 @@
>>> * @{
>>> */
>>>
>>> +#define CONFDB_DEFAULT_CFG_FILE_VER 2
>>> #define CONFDB_FILE "config.ldb"
>>> #define CONFDB_DEFAULT_CONFIG_FILE SSSD_CONF_DIR"/sssd.conf"
>>> #define SSSD_MIN_ID 1
>>> diff --git a/src/confdb/confdb_setup.c b/src/confdb/confdb_setup.c
>>> index 93a1a1b..360e3a8 100644
>>> --- a/src/confdb/confdb_setup.c
>>> +++ b/src/confdb/confdb_setup.c
>>> @@ -224,30 +224,30 @@ int confdb_init_db(const char *config_file, struct
confdb_ctx *cdb)
>>>
>>> ret = sss_ini_check_config_obj(init_data);
>>> if (ret != EOK) {
>>> - /* No known version. Assumed to be version 1 */
>>> - DEBUG(SSSDBG_FATAL_FAILURE,
>>> - "Config file is an old version. "
>>> - "Please run configuration upgrade script.\n");
>>> - ret = EINVAL;
>>> - goto done;
>>> - }
>>> -
>>> - version = sss_ini_get_int_config_value(init_data, 1, -1, &ret);
>>> - if (ret != EOK) {
>>> - DEBUG(SSSDBG_FATAL_FAILURE,
>>> - "Config file version could not be determined\n");
>>> - goto done;
>>> - } 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.
>>> + goto done;
>>> + } else if (version < CONFDB_VERSION_INT) {
>>> + DEBUG(SSSDBG_FATAL_FAILURE,
>>> + "Config file is an old version. "
>>> + "Please run configuration upgrade
script.\n");
^^^
and also here.
LS