> +-- | Gets the ids of nodes, instances, node groups,
> +--   networks, disks, nics, and the custer itself.
> +getAllIDs :: ConfigState -> [String]
> +getAllIDs cs =
> +  let lvs = getAllLVs cs
> [...]
> +  in nub $ lvs ++ instKeys ++ nodeKeys ++ instValues ++ nodeValues
> +         ++ nodeGroupValues ++ networkValues ++ disksValues ++ nics ++ 
> [cluster]

Don't use nub on lists that can get long; note that String is an instance
of Ord, hence non-quadratic algorithms can be used. Also, there is no
meaning of the order in this list---the only thing we care about is wether
a UUID is contained in it. Hence prefer sets from Data.Set.

This also applies to the other utility functions computing LVs, MACs, etc.

The rest of the patch looks OK (assuming that all the follow-up changes
are done).

That said, I still think, moving operations that actually modify the
configuration under WConfD's modifyConfigWithLock is a lot more important
than moving checks from the client to the already busy WConfD.

-- 
Klaus Aehlig
Google Germany GmbH, Dienerstr. 12, 80331 Muenchen
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschaeftsfuehrer: Graham Law, Christine Elizabeth Flores

Reply via email to