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<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 >> > >> >> >From f61ca88d69b3a56645b7473928b0674ac4bceff8 Mon Sep 17 00:00:00 2001 >> >From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@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 >> >--- >> * 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 _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel