Re: [SSSD] [PATCH] CONFDB: Assume config file version 2 if missing
On Mon, Sep 14, 2015 at 11:10:53AM +0200, Jakub Hrozek wrote: > CI had some issues, but I don't think those are related to your patch: > http://sssd-ci.duckdns.org/logs/job/26/52/summary.html > > ACK for sssd-1-12 * sssd-1-12: c35eb4aa64a67d34d343d608be40d60b61fb7d11 ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] CONFDB: Assume config file version 2 if missing
On Thu, Sep 03, 2015 at 02:29:16PM +0200, Michal Židek wrote:
> On 09/03/2015 09:58 AM, Lukas Slebodnik wrote:
> >On (03/09/15 09:35), Jakub Hrozek wrote:
> >>On Wed, Sep 02, 2015 at 02:52:42PM +0200, Lukas Slebodnik wrote:
> >>>On (01/09/15 12:55), Michal Židek wrote:
> 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.
> >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
>
>
Re: [SSSD] [PATCH] CONFDB: Assume config file version 2 if missing
On 09/03/2015 09:58 AM, Lukas Slebodnik wrote:
On (03/09/15 09:35), Jakub Hrozek wrote:
On Wed, Sep 02, 2015 at 02:52:42PM +0200, Lukas Slebodnik wrote:
On (01/09/15 12:55), Michal Židek wrote:
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.
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
>From f61ca88d69b3a56645b7473928b0674ac4bceff8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?=
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
---
* integration tests passed
* ipa-client generated config without config_file_version and sssd worked.
ACK
LS
* master: 175613be0cfb0890174d12d941e634d833b63dd9
Michal, can you rebase the patch for sssd-1-12 ?
The conflict might me caused by missing integration tests in 1.12
LS
Indeed it was the tests.
Patch for 1.12 is attached.
Michal
>From 38d413f01f160ee0083456f9ae17594ae67f0690 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?=
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/SSSDConfi
Re: [SSSD] [PATCH] CONFDB: Assume config file version 2 if missing
On (03/09/15 09:35), Jakub Hrozek wrote:
>On Wed, Sep 02, 2015 at 02:52:42PM +0200, Lukas Slebodnik wrote:
>> On (01/09/15 12:55), Michal Židek wrote:
>> >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.
>> >>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
>> >
>>
>> >From f61ca88d69b3a56645b7473928b0674ac4bceff8 Mon Sep 17 00:00:00 2001
>> >From: =?UTF-8?q?Michal=20=C5=BDidek?=
>> >Date: Tue, 7 Jul 2015 15:15:32 +0200
>> >Subject: [PATCH] CONFDB: Assume config file version 2 if missing
>>
Re: [SSSD] [PATCH] CONFDB: Assume config file version 2 if missing
On Wed, Sep 02, 2015 at 02:52:42PM +0200, Lukas Slebodnik wrote:
> On (01/09/15 12:55), Michal Židek wrote:
> >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.
> >>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
> >
>
> >From f61ca88d69b3a56645b7473928b0674ac4bceff8 Mon Sep 17 00:00:00 2001
> >From: =?UTF-8?q?Michal=20=C5=BDidek?=
> >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
> >---
Re: [SSSD] [PATCH] CONFDB: Assume config file version 2 if missing
On (01/09/15 12:55), Michal Židek wrote:
>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.
>>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
>
>From f61ca88d69b3a56645b7473928b0674ac4bceff8 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Michal=20=C5=BDidek?=
>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
>---
* integration tests passed
* ipa-client generated config without config_file_version and sssd worked.
ACK
LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] CONFDB: Assume config file version 2 if missing
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.
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
>From f61ca88d69b3a56645b7473928b0674ac4bceff8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?=
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/__init__.py.in | 5 ---
src/config/SSSDConfig/sssd_upgrade_config.py | 3 +-
src/config/SSSDConfigTest.py | 11 ++-
src/tests/intg/ldap_test.py | 2 --
src/tests/intg/test_memory_cache.py | 3 --
7 files changed, 29 insertions(+), 44 deletions(-)
diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 9aa2648..427c309 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..694a7f0 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 =
Re: [SSSD] [PATCH] CONFDB: Assume config file version 2 if missing
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.
> >> >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
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] CONFDB: Assume config file version 2 if missing
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.
>> >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
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] CONFDB: Assume config file version 2 if missing
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.
> >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?
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] CONFDB: Assume config file version 2 if missing
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.
>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
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] CONFDB: Assume config file version 2 if missing
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.
If I generate the config with realmd or ipa-client-install
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
little inconveniences.
Please also remove config_file_version from test_memory_cache.py
We do not necessary need to have all tests without
the option to test if the default works. But I removed
it from test_memory_cache.py anyway.
Michal
LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>From 2168de02177ffbd22281484aa8a0b974caca21c0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?=
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 +-
src/tests/intg/ldap_test.py | 2 --
src/tests/intg/test_memory_cache.py | 3 --
5 files changed, 27 insertions(+), 30 deletions(-)
diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index df45433..dece899 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..694a7f0 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,
Re: [SSSD] [PATCH] CONFDB: Assume config file version 2 if missing
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.
Please also remove config_file_version from test_memory_cache.py
LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] CONFDB: Assume config file version 2 if missing
On 07/30/2015 06:08 PM, Lukas Slebodnik wrote:
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?=
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?=
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.
you shoudl have proper indentation.
Fixed in the new version.
+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
>From f1de093f75bb35bb8768d3b612650891016c8d70 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?=
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 explicitl
Re: [SSSD] [PATCH] CONFDB: Assume config file version 2 if missing
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?=
>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?=
>>>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
you shoudl have proper indentation.
>>>+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
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fed
Re: [SSSD] [PATCH] CONFDB: Assume config file version 2 if missing
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?=
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?=
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");
+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;
+}
}
/* Set up a transaction to replace the configuration */
diff --git a/src/config/SSSDConfig/sssd_upgrade_config.py
b/src/config/SSSDConfig/sssd_upgrade_config.py
index 282d6c4..767d06d 100644
--- a/src/config/SSSDConfig/sssd_upgrade_config.py
+++ b/src/config/SSSDConfig/sssd_upgrade_config.py
@@ -47,7 +47,8 @@ class SSSDConfigFile(SSSDChangeConf):
def get_version(self):
ver = self.get_option_index('sssd', 'config_file_version')[1]
if not ver:
-return 1
+# config_file_ve
Re: [SSSD] [PATCH] CONFDB: Assume config file version 2 if missing
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?=
>>>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?=
>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");
>+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;
>+}
> }
>
> /* Set up a transaction to replace the configuration */
>diff --git a/src/config/SSSDConfig/sssd_upgrade_config.py
>b/src/config/SSSDConfig/sssd_upgrade_config.py
>index 282d6c4..767d06d 100644
>--- a/src/config/SSSDConfig/sssd_upgrade_config.py
>+++ b/src/config/SSSDConfig/sssd_upgrade_config.py
>@@ -47,7 +47,8 @@ class SSSDConfigFile(SSSDChangeConf):
>
Re: [SSSD] [PATCH] CONFDB: Assume config file version 2 if missing
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?=
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?=
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");
+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;
+}
}
/* Set up a transaction to replace the configuration */
diff --git a/src/config/SSSDConfig/sssd_upgrade_config.py b/src/config/SSSDConfig/sssd_upgrade_config.py
index 282d6c4..767d06d 100644
--- a/src/config/SSSDConfig/sssd_upgrade_config.py
+++ b/src/config/SSSDConfig/sssd_upgrade_config.py
@@ -47,7 +47,8 @@ class SSSDConfigFile(SSSDChangeConf):
def get_version(self):
ver = self.get_option_index('sssd', 'config_file_version')[1]
if not ver:
-return 1
+# config_file_version not found -> default to version 2
+return 2
try:
Re: [SSSD] [PATCH] CONFDB: Assume config file version 2 if missing
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?= >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 ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
