Recent changes to autorid (was Re: [SCM] Samba Shared Repository - branch master updated)

[Simo]

On Mon, 2014-04-28 at 11:48 +0200, Michael Adam wrote:
> On 2014-04-28 at 00:47 +0200, Michael Adam wrote:
> > On 2014-04-27 at 22:02 +0200, Christian Ambach wrote:
> > > Am 25.04.14 20:36, schrieb Simo:
> > > >On Fri, 2014-04-25 at 17:53 +0200, Michael Adam wrote:
> > > >>
> > > >>+#define IDMAP_AUTORID_ALLOC_RESERVED 500
> > > >
> > > >but I think this fiddling and making
> > > >strange exceptions with autorid is a bad idea.
> > >
> > > I somehow agree with Simo.
> > 
> > I had some good reasons for doing these changes
> > and doing them this way, but on the other hand I
> > certainly understand the bad feelings about it.
> > 
> > I will write a detailed explanation tomorrow
> > (it is too late now).
> 
> Ok, here we go:
> 
> The change that's met criticism is the introduction
> of a reserved area of a few (500) IDs at the top of
> the ALLOC range and a special treatment of the list
> of well-known SIDs that allocates these starting
> from the top of the range, moving down.
> 
> The reason for this change is the desire to have
> the mappings as deterministic as possible.
> 
> A problem with the well knowns when compared to
> other sids is that they don't share a common
> domain sid, but are rather a set of special domain
> SIDs (S-1-0, S-1-1, S-1-3, ...) with just a few
> valid rids, and also e.g. S-1-5 which is of course
> also the initial part of Builtin S-1-5-32 and
> of the domain sids S-1-5-21-...
> So an algorithmic treatment with the rid algorith
> would be difficult at best, and definitiely a waste
> of ranges. So a special treatment was considered needed.
> 
> The first attempt to make these determinstic was
> the addition of the function idmap_autorid_preallocate_wellknown()
> which is called from the initialization routine.
> This simply allocates the well-kowns in the ALLOC
> range, traversing a hard coded list. So the order
> of allocation determines the resulting mappings.
> 
> For a fresh install, this makes the mappings for
> these sids deterministic for a given configuration.
> (At least with my recent changes to wrap this
> into a transaction..)
> But for a DB that is upgraded, this may be different:
> If the old code had not used well-known allocation yet,
> but the alloc range had been used (as a source of
> IDs for group mapping), after the upgrade the well-knowns
> would be allocated starting at a higher GID than for the
> fresh install.
> 
> In order to make this more deterministic, various
> approaches were discussed (admittedly not on the list
> but directly with one major user of autorid..).
> One proposal was to use a separate special range
> and have a fixed calculation wellknown-sid<->xID
> in this range. But this was dismissed because one could
> also not guarantee a certain range for this, when it
> comes to upgrades. And it is very fragile since a careless
> change of the order of the sids in the code would break
> the DB in an upgrade and cause access-problems at least.
> 
> So we came up with this idea:
> We stick to the alloc range.
> But instead of using the regular gid HWM counter, we add a
> reserved sub-range at the top, which is filled starting
> at the top and counting down. The mapping is still
> determined by the order of the hard-coded list, but
> we achieve what I think is the best balance between
> determinism and maximum upgrade-compatibility and robustness
> because the mappings are stored as regular alloc-mappings:
> 
> - Case from above: if an older code did not map well-knowns,
>   the db upgraded from that code will have the exact
>   same well-known mappings as a fresh install with the
>   same configuration.
> 
> - If older code already allocated (some or all of) the
>   well-known sids, the upgraded code will honour these
>   and only allocate the missing ones in the upper subrange.
> 
> - If for some reason the 500 reserved IDs are not
>   enough, tose sids exceeding the 500 will simply be mapped
>   using the regular ALLOC mecahnism (below the top 500).
> 
> I agree that the code may appear strange at first glance,
> but the change is in fact rather clear and minimal imho:
> I have first reworked the code so that the flow is now
> rather clear:
> 
> sid-to-id:
>   if sid-is-for-alloc:
>     if sid-is-special (wellknown):
>       if special allocation in top range works:
>         done
>       end
>       # fall through to regular alloc..
>     end
>     regular alloc using HWM counters
>   else:
>     rid-based mapping
>   end
> end
> 
> Thanks to anyone who has read this far... ;-)

Thank you, but this is broken.
If you change the allocation range you are in the same problem as with
the old code, so I fail to see the utility of this patch.

I would rather say you ask people to crate a new small range (of 500
ids) for the "wellknown" domain and you are all set.
If it is available, on first init, you allocate all the wellknown ones
there in a predetermined order, if it is not you do as the old code did
unchanged for compatibility reasons.

In general I find that using alloc with autorid defeats the point of
using autorid, which is that of having always predictable mappings even
if they may be incomplete at times, so I do not agree very much with the
whole setting around this change.

> All that being said, I understand that the change might
> still be considered problematic. And I sitll hold up
> the offer to withdraw the decisive patches for further
> discussion.

> (Of course, I agree with Christian that this needs
>  some documentation in code and manpage. I will provide
>  those if we choose to keep the mechanism. Also then
>  we should of course add Christian's typo-fix.)
> 
> Opinions?

I think we should withdraw this approach, it makes things more
unpredictable, not less, IMHO.

Simo.