On 06/24/2016 02:52 PM, Lukas Slebodnik wrote:
> On (24/06/16 13:51), Lukas Slebodnik wrote:
> > On (24/06/16 13:49), Lukas Slebodnik wrote:
> > > On (24/06/16 10:29), Michal Židek wrote:
> > > > On 06/24/2016 09:57 AM, Jakub Hrozek wrote:
> > > > > On Thu, Jun 23, 2016 at 10:04:21PM +0200, Michal Židek wrote:
> > > > > > On 06/23/2016 09:40 PM, Jakub Hrozek wrote:
> > > > > > > On Thu, Jun 23, 2016 at 09:10:34PM +0200, Jakub Hrozek
wrote:
> > > > > > > > On Thu, Jun 23, 2016 at 07:29:13PM +0200, Lukas
Slebodnik wrote:
> > > > > > > > > On (23/06/16 18:59), Michal Židek wrote:
> > > > > > > > > > On 06/23/2016 06:52 PM, Lukas Slebodnik
wrote:
> > > > > > > > > > > On (23/06/16 18:24), Michal Židek
wrote:
> > > > > > > > > > > > On 06/23/2016 05:50 PM,
Michal Židek wrote:
> > > > > > > > > > > > > Lukas,
> > > > > > > > > > > > >
> > > > > > > > > > > > > did you have some local
patches related to
> > > > > > > > > > > > > this patch, that you did
not send here?
> > > > > > > > > > > > >
> > > > > > > > > > > > > Because I can not apply
the patch on current
> > > > > > > > > > > > > master and the changes
in the context of
> > > > > > > > > > > > > your patch seem like you
did something with
> > > > > > > > > > > > > default config file
path?
> > > > > > > > > > > > >
> > > > > > > > > > > > > Slightly modified
version that applies on
> > > > > > > > > > > > > current master is
attached (the changes
> > > > > > > > > > > > > are the same, but the
context differs).
> > > > > > > > > > > > >
> > > > > > > > > > > > > Michal
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > I added the missing access
check for
> > > > > > > > > > > > snippets. If the snippets
does not
> > > > > > > > > > > > pass the access check it is
skipped
> > > > > > > > > > > > and a loud debug message is
logged.
> > > > > > > > > > > >
> > > > > > > > > > > > I attach it as separate patch
for
> > > > > > > > > > > > easier review, but it should
be squashed
> > > > > > > > > > > > before pushing. I also attach
the first
> > > > > > > > > > > > patch for convenience.
> > > > > > > > > > > >
> > > > > > > > > > > > Michal
> > > > > > > > > > >
> > > > > > > > > > > > From
6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 00:00:00 2001
> > > > > > > > > > > > From:
=?UTF-8?q?Michal=20=C5=BDidek?= <mzidek(a)redhat.com>
> > > > > > > > > > > > Date: Tue, 22 Mar 2016
14:09:34 +0100
> > > > > > > > > > > > Subject: [PATCH] confdb: Make
it possible to use config snippets
> > > > > > > > > > > >
> > > > > > > > > > > > Resolves:
> > > > > > > > > > > >
https://fedorahosted.org/sssd/ticket/2247
> > > > > > > > > > > >
> > > > > > > > > > > > Signed-off-by: Lukas
Slebodnik <lslebodn(a)redhat.com>
> > > > > > > > > > > > ---
> > > > > > > > > LGTM.
> > > > > > > > >
> > > > > > > > > It isn't ACK because I was involved in
development
> > > > > > > > > and I didn;t test last version but code is
fine fore me.
> > > > > > > >
> > > > > > > > I just ran a quick test and the patch works for
me and the code reads OK
> > > > > > > > as well. I will push after CI finishes.
> > > > > > > >
> > > > > > > > I also opened:
> > > > > > > >
https://fedorahosted.org/sssd/ticket/3062
> > > > > > > > because the patch is missing documentation and
> > > > > > > >
https://fedorahosted.org/sssd/ticket/3063
> > > > > > > > to add at least a simple integration test.
> > > > > > >
> > > > > > > Actually, can we get rid of this warning?
> > > > > > >
> > > > > > > Error: COMPILER_WARNING:
> > > > > > > sssd-1.13.91/src/util/sss_ini.c:216:17: warning:
unused variable 'patterns' [-Wunused-variable]
> > > > > > > # const char *patterns[] = {
"^[^\\.].*\\.conf", NULL };
> > > > > > > # ^
> > > > > > > # 214| int ret;
> > > > > > > # 215| #ifdef HAVE_LIBINI_CONFIG_V1
> > > > > > > # 216|-> const char *patterns[] = {
"^[^\\.].*\\.conf", NULL };
> > > > > > > # 217| const char *sections[] = {
".*", NULL };
> > > > > > > # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3
> > > > > > >
> > > > > > > Error: COMPILER_WARNING:
> > > > > > > sssd-1.13.91/src/util/sss_ini.c: scope_hint: In
function 'sss_ini_get_config'
> > > > > > > sssd-1.13.91/src/util/sss_ini.c:217:17: warning:
unused variable 'sections' [-Wunused-variable]
> > > > > > > # const char *sections[] = { ".*", NULL
};
> > > > > > > # ^
> > > > > > > # 215| #ifdef HAVE_LIBINI_CONFIG_V1
> > > > > > > # 216| const char *patterns[] = {
"^[^\\.].*\\.conf", NULL };
> > > > > > > # 217|-> const char *sections[] = {
".*", NULL };
> > > > > > > # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3
> > > > > > > # 219| uint32_t i = 0;
> > > > > > > _______________________________________________
> > > > > >
> > > > > > New patch attached.
> > > > > >
> > > > > > Michal
> > > > > >
> > > > >
> > > > > The patch is fine, (so ack also from me), but can someone check
if the
> > > > > attached rebase on top of Pavel's sssctl is OK?
> > > > >
> > > > >
> > > >
> > > > I did not check the differences between the rebased and
> > > > non-rebased version, but I just tried branch with sssctl
> > > > patches + this rebased patch + startup validation patches
> > > > and it worked for me.
> > > >
> > > > But the CI failed for me with the snippets in place.
> > > > Will take a look at what is happening there.
> > > >
> > > It failed beccause it bas broken with libini_config 1.0 .. 1.2
> > > and current CI machines still have libini_config 1.2
> > > or 1.1 (el6)
> > >
> > > Updated patch is attached.
> > >
> > Ups,
> >
> > ENOPATCH
> >
> > fixed in this version :-)
> >
> Previous version had an issue with SELinux and blocking
> access to conf.d.
>
> Hopefully last version is attached.
>
> LS
>
Thanks, this version works for all CI machines.
Here is result of the CI:
http://sssd-ci.duckdns.org/logs/job/46/08/summary.html
There are two failures that are not related
to these patches (One is "device is busy" error during
mock and the other was that ldap server failed to start
for one set of tests).
I would like to push the sssctl patches first, can you rebase? (You can
see what I changed in the previous mail to rebase the patch atop
sssctl).