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