[pve-devel] [PATCH container] lxc start: warn in case of conflicting lxc.idmap entries

[Friedrich Weber]

Thanks for the review!

On 16/03/2023 14:59, Wolfgang Bumiller wrote:
> Both seem a bit excessive to me.
> 
> Let's look at the data:
> We have a set of ranges consisting of a type, 2 starts and a count.
> The types are uids and gids, so we can view those as 2 separate
> instances of sets of [ct_start, host_start, count].
> Since neither the container nor the host sides must overlap we can -
> again - view these as separate sets of container side [start, count] and
> host side [start, count].
> In other words, we can see the entire id map as just 4 sets of [start,
> count] ranges which must not overlap.
> 
> So I think all we need to do is sort these by the 'start' value, and for
> each element make sure that
> 
>      prevous_start + previous_count <= current_start
> 
> And yes, that means we need to sort $id_maps twice, once by ct id, once
> by host id, and then iterate and do the above check.
> 
> Should be much shorter (and faster).

Yeah, good point, splitting $id_maps into separate uid/gid maps, and 
then sorting+iterating twice (I'll call this the "sorting algorithm" 
below) does sound more understandable than the current ad-hoc approach, 
and faster too.

However, one small benefit of iterating over $id_maps in its original 
order (instead of sorting) is that the error message always references 
the *first* invalid map entry in the config, e.g. (omitting host uids 
for clarity)

   1) u 1000 <...> 100
   2) u 950 <...> 100
   3) u 900 <...> 100
   4) u 850 <...> 100

The sorting algorithm would error on entry 3, which might suggest to 
users that entries 1-2 are okay (which they are not). The current 
algorithm errors on line 2 already. Similar things would happen with 
interleaved uid/gid mappings, I guess.

But I'm not sure if this really matters to users. What do you think?