yet another idmap rewrite - still for 3.6 ?
Michael Adam
obnox at samba.org
Fri Jul 30 09:47:25 MDT 2010
Hi,
as many of you know, I have been working hard on YAIR -- yet
another idmap rewrite for samba3.
I have already talked about the idea and beginnings at sambaXP
2009, and the general direction was agreed upon, but got slowed
down by lack of time and some technical points. I have now
recently picked up this rewrite again and finished a first stage.
The big benefit was that by introducing atomicity into the
idmapping operations, I could achieve performance improvements
of creation if id mappings in a ctdb/samba cluster.
But I have pursued the general API cleanup presented in the talk.
This mail is to request that this code still gets into the
3.6 release, even though I did not manage to polish my
patchset before the pre1 release.
Here are the details of why and how:
YAIR - Yet Another Idmap Rewrite
=================================
Deficiencies of the current system:
-----------------------------------
In the Simo's Idmap rewrite of 3.0.25 that introduced
the support for multiple idmap backends and support for
configuring different domains with different backends
and ranges explicitly, two key design decisions were
made, related to the separation of idmap backends in
"allocating" backends like tdb and ldap (that can create
new Unix IDs and new ID mappings on their own) and non-
allocating backends like rid and ad:
1. The allocator was separated from the actual id mapping.
This was partly due to the fact that the id allocator
is used in group mapping and ldapsam:editposix code, too,
and hence needed to be accessible from outside the idmap
system.
One consequence of this was the slightly confusing of
the allocation config on the surface in smb.conf.
This added the very flexible possiblity to e.g. use
an ldap id allocator to store id mappings in tdb.
Or vice versa.
2. There was to be only one allocator and only one
domain could essentially be configured in smb.conf
to use an allocating backend.
Volker's rewrite for 3.3.0 in 2008 simplified the configuration
a bit but did not fundamentally change this design.
There are several downsides of the design:
1. The configuration is pretty complex and was rather confusing.
2. API-wise the separation of allocation of ids and storing of
mappings meant that the methods for storing mappings needed
to be present on the top level.
The id mapping API should just consist of sids_to_unixids()
and unixids_to_sids(). Instead, there were also
set_mapping(), remove_mapping() and a utility mehtod dump_data().
The idmap_alloc_api consists of:
allocate_id(), get_id_hwm(), set_id_hwm().
The pragmatic downside of the second item is that the logic that checks
for the existence of a mapping, and if none is found gets a new id
and then stores the mapping had to be at a faily high level.
Basically direclty called from winbindd. So sids_to_unixids was not
atomic. It was in particular not possible to wrap it inside a transaction
(for tdb and tdb2 backends).
This recently bit us pretty badly, performance-wise in a cluster setup
(ctdb) with the tdb2 backend.
YAIR - Ideas:
-------------
Already at sambaXP 2009, I have proposed (and had already started)
another rewrite of the ID mapping system that is based on the following
design ideas:
(http://samba.org/~obnox/presentations/sambaXP-2009/sambaxp-2009-talk-obnox-slides-paper.pdf)
1. The id mapping API should just consist of the methods
- sids_to_unixids
- unixids_to_sids
These calls should be atomic and the backend should know by itself
whether it needs to allocate some ids, store mappings and how.
To the caller, this should be completely irrelevant.
2. The idmap_alloc_api should, consequently vanish completely.
This would sacrifice some amount of flexibility, namely the
possibility to use a different allocator for an idmap backend.
But it would gain greater flexibility in that it would allow
the backends to be completely self-sufficient in implementing
their allocators and allow for instance to configure various
"allocating" idmap domains with different ranges and backends.
The project came to a halt due to lack of time and the fact that
I only realized at that point that the idmap_alloc code was not
only an id allocator for the id mapping subsystem but also
for all of samba (group mapping, editposix...). So I had to stop
and think about it.
First stage competed:
---------------------
Recently, due to the need for better idmap performance in the cluster,
I picked up the project again and carried it through to a certain
extent.
I have now completed a first step that achieved the following:
1. The idmap alloc backend code and api have completely vanished.
In particular the "idmap alloc backend" parameter has vanished.
2. the idmap api now consists of
- sids_to_unixids
- unixids_to_sids
- allocate_id
I.e., as a compromise until we have a better solution, the allocate_id
functionality is preserved, but via the idmap api now, so that group mapping
and ldapsam:editposix can still work.
(This allocator only works through the default idmap config.)
3. Since all the methods have vanished from idmap's surface, they are also
no longer available through winbindd. The following winbind methods
have been removed:
- set_mapping
- remove_mapping
- set_hwm
The allocate_uid and allocate_gid calls have been preserved for now
for simplicity. But calling into idmap methods as detailed above.
4. Since winbindd is no longer offering the corresponding calls,
"net idmap restore" has been rewritten to directly write mappings
into the database. This has been done for the tdb and tdb2 backends.
ldap needs to follow it desired.
net idmap dump/restore have been written so that they use dbrwrap
now. This means that they are now also useful in a cluster which
they weren't before.
In this first step, the current functionality has been kept as far as
possible. New functionality has not been added yet, e.g. Allocators
for individual domains. This means that still (as in versions since
3.0.25) only the default idmap config does allocation of new ids
(if it is using an allocating backend). This is something that can
be extended as needed in future steps on a per-backend basis.
The main focus has been on unifiying and cleaning code and apis.
* the idmap.c code has been greatly cleaned (idmap_alloc stuff gone,
alloc/set-mapping logic gone into the backends).
* Some code common to all backends has been refactored out into
idmap.c, notably the parsing of common config parts (range,
and a new read_only parameter) into the central idmap_domain
struct instead of individual almost identical code in the backends.
* The backend code has been greatly unified.
Especially the tdb and tdb2 backends are now again almost identical.
The only differences are:
- the tdb upgrade code in the tdb backend only
- the idmap:script code in the tdb2 backend only
These two could hence be unified.
* the ldap backend has kept its special config options which
still use the "idmap alloc config DOMAIN : " prefix for
determining the dn and so for the allocator in order to be
as little disruptive as possible for now. But this should
definitely be unified as (e.g.)
"idmap config DOMAIN : alloc base_db = ..."
These changes have already gone into the v3-4-ctdb clustered
samba branch and have been successfully tested there, giving
2-3 times better idmap performance due to atomicity.
I have now taken great pains to polish and adapt the patchset
for master. I would really love to see this go into 3.6.0.
The code can bee seen in my master-idmap-wip repositor under
git://git.samba.org/obnox/samba/samba-obnox.git
gitweb:
http://git.samba.org/?p=obnox/samba/samba-obnox.git;a=shortlog;h=refs/heads/master-idmap-wip
The diffstat against master this:
docs-xml/manpages-3/net.8.xml | 2 +-
docs-xml/smbdotconf/winbind/idmapallocbackend.xml | 33 -
docs-xml/smbdotconf/winbind/idmapreadonly.xml | 21 +
nsswitch/libwbclient/wbc_idmap.c | 186 +----
nsswitch/winbind_struct_protocol.h | 11 +-
source3/Makefile.in | 15 +-
source3/groupdb/mapping.c | 13 +-
source3/include/idmap.h | 24 +-
source3/include/proto.h | 10 +-
source3/librpc/idl/wbint.idl | 23 -
source3/param/loadparm.c | 11 +-
source3/passdb/pdb_ldap.c | 47 +-
source3/utils/net_idmap.c | 271 ++++---
source3/utils/net_sam.c | 17 +-
source3/winbindd/idmap.c | 413 +++-------
source3/winbindd/idmap_ad.c | 32 +-
source3/winbindd/idmap_adex/idmap_adex.c | 38 -
source3/winbindd/idmap_ldap.c | 784 ++++++-----------
source3/winbindd/idmap_rid.c | 105 +--
source3/winbindd/idmap_rw.c | 79 ++
source3/winbindd/idmap_rw.h | 56 ++
source3/winbindd/idmap_tdb.c | 1011 ++++++++-------------
source3/winbindd/idmap_tdb2.c | 638 ++++++--------
source3/winbindd/idmap_util.c | 102 ++-
source3/winbindd/winbindd.c | 6 -
source3/winbindd/winbindd_dual_srv.c | 68 --
source3/winbindd/winbindd_proto.h | 21 -
source3/winbindd/winbindd_remove_mapping.c | 106 ---
source3/winbindd/winbindd_set_hwm.c | 95 --
source3/winbindd/winbindd_set_mapping.c | 106 ---
source4/winbind/wb_samba3_protocol.c | 6 -
31 files changed, 1484 insertions(+), 2866 deletions(-)
I would like to hear your opinions about this project as such
and regarding inclusion into 3.6.
Cheers - Michael
PS: And here is the current git lo origin/master..HEAD :
aa012dcd s3:idmap_ldap: use idmap_rw_new_mapping in idmap_ldap_new_mapping
d1e12a19 s3:idmap_ldap: add idmap_rw_ops to idmap_ldap_context and init in db_init()
f1b97f52 s3:idmap_tdb: use idmap_rw_new_mapping in idmap_tdb_new_mapping
df50e542 s3:idmap_tdb: add idmap_rw_ops to idmap_tdb_context and initialize them in init_db
5c8475da s3:idmap_tdb2: use idmap_rw_new_mapping in idmap_tdb2_new_mapping
b34b7dd2 s3:idmap_tdb2: add rw_ops to idmap_tdb2_context and initialize in idmap_tdb2_db_init
c3923b7e s3:idmap: add abstract idmap_rw new_mapping mechanism without registering backends
32ffa492 s3:idmap_ad: untangle two assignments from checks
ceac573b s3:idmap_ad: remove unused filter_low_id and filter_high_id from idmap_ad_context
104de004 s3:idmap_ad: use range from idmap_domain in idmap_ad_sids_to_unixids()
5445d07b s3:idmap_ad: use range from idmap_domain in idmap_ad_unixids_to_sids()
d4879596 s3:idmap_rid: remove a comment that does not apply in that place.
025f0f33 s3:idmap_rid: remove unused domain_name from the idmap_rid_context.
85a7a0c4 s3:idmap_rid: remove range from idmap_rid_context()
419c86df s3:idmap_rid: use range from idmap_domain in idmap_rid_sid_to_id()
9a37693e s3:idmap_rid: use ranges from idmap_domain struct in idmap_rid_id_to_sid()
2643c3af s3:idmap_rid: remove unused talloc context var from idmap_rid_sids_to_unixids()
d2ca458e s3:idmap_rid: remove unused talloc context arg from idmap_rid_sid_to_id()
4de4d6b9 s3:idmap_rid: remove unused talloc context var from idmap_rid_unixids_to_sids()
cd59ab83 s3:idmap_rid: remove unused talloc ctx argument from idmap_rid_id_to_sid()
b1e36c35 s3:idmap_rid: untangle assignment from check in idmap_rid_initialize()
6e6ceb02 s3:idmap_ldap: add my (C)
322d30d4 s3:idmap_ldap: create mappings for unmapped sids in idmap_ldap_sids_to_unixids()
48978cae s3:idmap_ldap: add a idmap_ldap_new_mapping().
3bb30764 s3:idmap_ldap: add idmap_ldap_get_new_id() to allocate a new id given a domain
b065d06d s3:idmap_ldap: move idmap_ldap_set_mapping() further up.
39e8f0ad s3:idmap_ldap: make idmap_ldap_alloc_context a member of idmap_ldap_context
b5a62bab s3:idmap_ldap: call idmap_ldap_alloc_init from idmap_ldap_init.
386a2623 s3:idmap_ldap: remove the (now unused) range from idmap_ldap_alloc_context
570a5ad1 s3:idmap_ldap: use ranges from idmap domain in idmap_ldap_allocate_id()
28666175 s3:idmap_ldap: add idmap_domain arg to idmap_ldap_alloc_init and verify_idpool
fc2a61c1 s3:idmap_ldap: remove unused filter range from struct idmap_ldap_context
1305c09c s3:idmap_ldap: don't load ranges - they have been loaded into struct idmap_domain
dad13f0c s3:idmap_ldap: use filter range from idmap domain, not idmap_ldap_context
fa059a67 s3:idmap_ldap: re-implement allocate_id in idmap methods.
ff07b2ac s3:idmap_tdb: add my (C)
662e298a s3:idmap_tdb: properly initialize the idmap_tdb context with zero
5fa9b3c4 s3:idmap_tdb: prevent opening the idmap db more than once.
6795ecf0 s3:idmap_tdb: rewrite sids_to_unixids to create mappings for unmapped sids.
4efb49e3 s3:idmap_tdb: add a idmap_tdb_new_mapping().
b0114f9f s3:idmap_tdb: move the set_mapping code up
d73e1c6c s3:idmap_tdb: use transaction wrapper for idmap_tdb_set_mapping().
77fe2881 s3:idmap_tdb: remove unused struct idmap_tdb_state.
cfdd638f s3:idmap_tdb: remove unused idmap_alloc_db
788a33bc s3:idmap_tdb: remove unused idmap_tdb_alloc_close().
31b383f6 s3:idmap_tdb: give idmap_domain arg to idmap_tdb_allocate_id and use ctx->db
a47c9a29 s3:idmap_tdb: call idmap_tdb_init_hwm() from idmap_tdb_open_db().
90d63018 s3:idmap_tdb: move idmap_tdb_init_hwm up.
bf489f14 s3:idmap_tdb: remove unused idmap_tdb_load_ranges()
2edde9b8 s3:idmap_tdb: have idmap_tdb_open_db take an idmap_domain struct as argument
05933602 s3:idmap_tdb: rename idmap_tdb_alloc_init->idmap_tdb_init_hwm and use db from idmap_tdb_context
293fe83b s3:idmap_tdb: move definition of struct idmap_tdb_context up.
ea3d582c s3:idmap_tdb: remove filter_low_id,filter_high_id from idmap_tdb_context
3ce5e6b3 s3:idmap_tdb: add idmap domain arg to idmap_tdb_upgrade and use domain range
3e693303 s3:idmap_tdb: use filter from idmap_domain rather than from idmap_tdb_context
5ecd1a69 s3:idmap_tdb: give idmap domain argument to idmap_tdb_sid_to_id
d3e49ac3 s3:idmap_tdb: give idmap domain argument to idmap_tdb_id_to_sid
2fea86b8 s3:idmap_tdb: implement allocate_id in idmap methods for tdb backend
6f0b999d s3:idmap_tdb: add idmap_tdb_get_new_id() to allocate a new id given a domain
15b61667 s3:idmap_tdb: convert idmap_tdb_allocate_id() to use transaction wrappers
66f26077 s3:idmap_tdb: remove an extra blank line
b69b2ccc s3:idmap_tdb2: add my (C)
49701626 s3:idmap_tdb2: move idmap_tdb2_new_mapping() up. spare a prototype.
10eecfa0 s3:idmap_tdb2: get rid of an extra variable in idmap_tdb2_db_init().
3d195011 s3:idmap_tdb2: move idmap_tdb2_set_mapping() up to its _action callback.
9a458af4 s3:idmap_tdb2: use the right talloc context for db_open in idmap_tdb2_open_db()
210120b7 s3:idmap_tdb2: don't check whether sid is already mapped in idmap_tdb2_new_mapping().
305ce359 s3:idmap_tdb2: add the db_context to the idmap_tdb2_context
1e1b4789 s3:idmap_tdb2: talloc_zero (instead of talloc) the idmap_tdb2_context
2dc125f9 s3:idmap_tdb2: rename idmap_tdb2_alloc_load -> idmap_tdb2_init_hwm
56cd1fd7 s3:idmap_tdb2: move idmap_tdb2_alloc_load() up to reduce need for prototype
70da7dfc s3:idmap_tdb2: remove unused idmap_tdb2_state and idmap_tdb2_load_ranges
7014cd74 s3:idmap_tdb2: give idmap_tdb2_alloc_load() and idmap domain arguemnt
c494b000 s3:idmap_tdb2: add an idmap_domain struct argument to idmap_tdb2_open_db()
cccef7af s3:idmap_tdb2: remove filter_low_id and filter_high_id from idmap_tdb2_context
8ce1ab00 s3:idmap_tdb2: don't parse config and fill filter_low_id and filter_high_id
9410653f s3:idmap_tdb2: honour the "idmap read only" flag in the tdb2 module.
4b808fb4 s3:idmap_tdb2: use range from idmap_domain in idmap_tdb2_allocate_id
0c0dfd64 s3:idmap_tdb2: use filter from idmap_domain rather than from idmap_tdb2_context
4cdfae81 s3:idmap_tdb2: pass idmap_domain (not idmap_tdb2_context) to idmap_tdb2_sid_to_id
0da6d163 s3:idmap_tdb2: pass idmap_domain instead of idmap_tdb2_context to idmap_tdb2_unixids_to_sids
85eae33c s3:idmap_tdb2: also support idmap script for named domains
8b051038 s3:idmap_tdb2: move the idmap script from idmap_tdb2_state to idmap_tdb2_context
fa12beb2 s3:idmap_tdb2: remove use of idmap_tdb2_state from idmap_tdb2_allocate_id
552a549e s3:idmap_tdb2: move definition of struct idmap_tdb2_context up.
c22a7736 s3:idmap_tdb2: open the db after loading the ranges in idmap_tdb2_db_init().
321c98df s3:idmap_tdb2: add allocation of new mappings to idmap_tdb2_sids_to_unixids
6cf8a307 s3:idmap_tdb2: re-implement allocated_id in idmap methods.
5a71df83 s3:idmap: add idmap_unix_id_is_in_range() for checking an id against an idmap range
5c9a5be5 s3:idmap: don't check range for passdb idmap domain
d2973882 s3:idmap: parse ranges and "read only" in idmap_init_domain().
f1e09aa3 s3:idmap: add a read_only flag to the idmap_domain struct.
73ca27c4 s3:idmap: add low_id and high_id to the idmap_domain struct
4c28cda6 s3:docs: fix net manpage to reflect removal of net "idmap secret alloc" feature
73066c2a s3:net: remove the "net idmap secret alloc" functionality.
cf37a738 s3:docs: add documentation for new "idmap read only" parameter
43b0b08b s3:loadparm: add new boolean parameter "idmap read only"
5ee94771 s3:docs: remove documentation of removed parameter "idmap alloc backend"
11fb67c2 s3:loadparm: remove parameter "idmap alloc backend"
038ac064 s3:idmap: remove unused definition of idmap_alloc_methods.
0e8c48d8 s3:idmap: remove idmap_alloc_context from idmap.c
2c860f4a s3:idmap: remove the alloc methods list from idmap.c
68bb8a92 s3:idmap: remove unused get_alloc_methods().
e9078a84 s3:idmap: remove unused smb_register_idmap_alloc().
818b13dd s3:idmap_ldap: remove unused idmap_ldap_alloc_methods.
79f77a60 s3:idmap_ldap: remoce unused idmap_alloc_ldap_init
23257547 s3:idmap_ldap: don't call idmap_alloc_ldap_init in idmap_ldap_init
e16363e6 s3:idmap_tdb: remove unused idmap_alloc_methods
22b194d2 s3:idmap_tdb: remove unused idmap_alloc_tdb_init()
b251ad1e s3:idmap_tdb: don't call idmap_alloc_tdb_init in idmap_tdb_init
c4554bdf s3:idmap_tdb2: remove unused idmap_tdb2_alloc_init().
767515ce s3:idmap_tdb2: remove unused idmap_tdb2_alloc_close().
6d4f6d9b s3:idmap_tdb2: remove unused idmap_alloc_methods.
3b8b1082 s3:idmap_tdb2: don't call smb_register_idmap_alloc() in idmap_tdb2_init
05da1e3a s3:idmap_tdb: make idmap_alloc_tdb_init() static.
70b527ad s3:idmap: remove unused idmap_alloc_init().
b4e0dc85 s3:idmap: use allocate_id() from the idmap_methods in idmap_allocate_unixid()
15f3c578 s3:idmap: add an allocate_id method to the idmap_methods struct.
8ddd7c57 s3:idmap: factor out common code of idmap_allocate_uid|gid()
868886c2 s3:idmap: remove the set_mapping method from the idmap API
fa94a45f s3:idmap: remove idmap_new_mapping() - now implemented in the backends
b7440365 s3:idmap: add a debug message to idmap_sid_to_gid
63d8fd22 s3:idmap: add a debug message to idmap_sid_to_uid
82656d23 s3:idmap: don't call idmap_new_mapping idmap_sid_to_gid
2b8341f9 s3:idmap: don't call idmap_new_mapping idmap_sid_to_unixid.
34a71f03 s3:winbind: increase interface version after removing calls
d3056a54 s3:idmap: remove unused method set_id_hwm from idmap API
b2baf413 s3:idmap: remove unused alloc method get_id_hwm from idmap API
52563545 s3:idmap: remove unused method dump_data() from the idmap API
50d1026d s3:idmap: remove the remove_mapping method from API and backends
60eb6053 s3:idmap: remove unused idmap_remove_mapping().
da5ce850 s4: remove REMOVE_MAPPING from wb_samba3_protocol
f5507e9c s3:winbind: remove the method REMOVE_MAPPING from winbind's API
1232e90e s3:idmap: remove unused idmap_set_mapping().
f9241ed8 s4: remove SET_MAPPING from wb_samba3_protocol
912c0443 s3:winbind: remove the method SET_MAPPING from winbind's API
9f9b8a6b libwbclient: unimplement wbcRemoveGidMapping()
ba570c06 libwbclient: unimplement wbcRemoveUidMapping()
506508e0 libwbclient: unimplement wbcSetGidMapping()
18b1b1c3 libwbclient: unimplement wbcSetUidMapping()
cdbb922f s3:idmap: remove unused idmap_set_gid_hwm()
2a4cec0b s3:idmap: remove unused idmap_set_uid_hwm()
ab6873a3 s4: remove SET_HWM and SET_DUAL_HWM from wb_samba3_protocol
faba68d0 s3:winbind: remove SET_HWM from winbind's API.
c61fb9e7 libwbclient: unimplement wbcSetGidHwm()
57910e67 libwbclient: unimplement wbcSetUidHwm()
02a4783c s3:net: rewrite "net idmap restore" using dbwrap
e829fb69 s3:net: change "net idmap dump" to use dbwrap instead of direct tdb access
fd97a992 s3:net sam provision: compose sid before getting uid for Guest
390b2e2b s3:net sam provision: allocate the uid after composing the sid for Administrator
b7727ddd s3:pdb_ldap: move some code in ldapsam_create_dom_group()
15b9a939 s3:groupdb: allocate a gid after allocating a rid in pdb_default_create_alias()
534440b5 s3:idmap_tdb2: fix a debug message
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 206 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20100730/291b6142/attachment.pgp>
More information about the samba-technical
mailing list