On (14/11/13 18:41), Lukas Slebodnik wrote:
On (12/11/13 12:18), Michal Židek wrote:
>On 11/09/2013 06:07 AM, Simo Sorce wrote:
>>On Fri, 2013-11-08 at 16:41 +0100, Michal Židek wrote:
>>>Hello,
>>>
>>>these patches (made by Jakub and me) remove redundant sysdb_ctx
>>>parameter from sysdb API in functions that are closely bound to some
>>>domain (sss_domain_info already contains sysdb_ctx, so there is no need
>>>to pass it as a separate parameter).
>>
>>Looks ok, but please separate the patches that fix the names (last one
>>of the set) and one patch that includes removeal of unused functions in
>>separate patchset. Keep this one exclusively to handle the removal and
>>don't be tempted to sneak in any other change, because those will get
>>lost in the noise.
>
>Ok. I will send the patch with the parameter name in a different thread
>after this lands in master. But the patch that merges
>ipa_selinux_common.c and ipa_selinux.c files is required to apply the
>rest of the patches so I kept it here, and just split into separate
>patch.
>
>>
>>>NOTE:
>>>This is the first wave of sysdb refactoring effort. Other sysdb changes
>>>will follow, including ticket
>>>https://fedorahosted.org/sssd/ticket/2129
>>>(Always store users with FQDN with hardcoded format), which is change
>>>that should simplify handling of 'name' attribute and avoid code
>>>patterns like this:
>>>
>>>if (IS_SUBDOMAIN) {
>>> parse name and domain part from 'name' attribute using
>>> regular expression (requires initialization of names_ctx).
>>>} else {
>>> 'name' attribute contains only name
>>>}
>>>
>>>In other words, we should store data in unified manner for main domains
>>>as well as for subdomains and avoid the need for regular expressions
>>>when simply need to parse name and domain portion from the 'name'
>>>attribute. Alternatively we might store name and domain portions in
>>>separate attributes instead of one FQDN attribute.
>>
>>Separate attributes looks like it would be more robust, but then you
>>have to deal with potential name conflicts with subdomains, so you may
>>have little choice here, short of creating subtrees for subdomains.
>>
>>Have you already thought about how to upgrade an existing DB ?
>>I am a bit concerned about all the renames, it may take a *lot* of time
>>if done at startup, and the DB is big, as a lot of indexes needs to be
>>fixed. Perhaps we can drop all indeces, do all renames and then rebuild
>>indeces ...
>
>I thought it is not a big deal since it happens only once after
>upgrade of SSSD. Maybe just syslog with message something like
>'Converting databese to new sysdb format version. This may take some
>time depending on the size of the database.'... or something like
>that.
>
>>
>>Simo.
>>
>
>New patches attached.
>
>Michal
>From 26cbac496383e7925f797db73c324f029352e2e3 Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek(a)redhat.com>
>Date: Tue, 12 Nov 2013 11:08:12 +0100
>Subject: [PATCH 01/12] Merge ipa_selinux_common.c and ipa_selinux.c
>
>Moved unused functions and merged ipa_selinux_common.c into
>ipa_selinux.c
>---
ACK
I tested also patch set "00\d\d-SYSDB-Drop*" and I didn't found any
unexpected
behaviour. I think it is because unit tests for sysdb cover a lot of use cases.
ACK for patch set.
LS
0010-SYSDB-Drop-the-sysdb_ctx-parameter-module-sysdb_ops-.patch
^^^^^^
This patch broke the compilation of sssd
CC src/db/sysdb_ops.lo
src/db/sysdb_ops.c:444:61: error: too many arguments to function call, expected 3, have 4
basedn = sysdb_group_dn(domain->sysdb, tmp_ctx, domain, name);
~~~~~~~~~~~~~~ ^~~~
src/db/sysdb.h:336:1: note: 'sysdb_group_dn' declared here
struct ldb_dn *sysdb_group_dn(TALLOC_CTX *mem_ctx, struct sss_domain_info *dom,
^
src/db/sysdb_ops.c:559:64: error: too many arguments to function call, expected 3, have 4
basedn = sysdb_netgroup_dn(domain->sysdb, tmp_ctx, domain, name);
~~~~~~~~~~~~~~~~~ ^~~~
src/db/sysdb.h:338:1: note: 'sysdb_netgroup_dn' declared here
struct ldb_dn *sysdb_netgroup_dn(TALLOC_CTX *mem_ctx,
^
src/db/sysdb_ops.c:664:56: error: too many arguments to function call, expected 3, have 4
dn = sysdb_user_dn(domain->sysdb, tmp_ctx, domain, name);
~~~~~~~~~~~~~ ^~~~
src/db/sysdb.h:334:1: note: 'sysdb_user_dn' declared here
struct ldb_dn *sysdb_user_dn(TALLOC_CTX *mem_ctx, struct sss_domain_info *dom,
^
src/db/sysdb_ops.c:699:57: error: too many arguments to function call, expected 3, have 4
dn = sysdb_group_dn(domain->sysdb, tmp_ctx, domain, name);
~~~~~~~~~~~~~~ ^~~~
src/db/sysdb.h:336:1: note: 'sysdb_group_dn' declared here
struct ldb_dn *sysdb_group_dn(TALLOC_CTX *mem_ctx, struct sss_domain_info *dom,
and lot of another errors
0011-SYSDB-Drop-the-sysdb_ctx-parameter-module-sysdb_ops-.patch
^^^^^
This patch fixed this problem
LS