I believe that the logic of this function is correct, but reading this code is painful enough that it might be very hard to maintain in the future.
Some concrete suggestions on improving this: Within the function, i is used as a prefix indicating both instance and initial, and it is also the first letter of two other variables - inst and idx. Could this somehow be improved? I could not decipher the naming behind afn, and this is partially due to the name being used only once in the rest of the code, and partially due to the fn root having another expected meaning in Haskell. Can we get a better name or comment? Thanks! On Thu, Nov 14, 2013 at 11:06 PM, Klaus Aehlig <[email protected]> wrote: > Add a function that, given two adjacent cluster configurations of > a balancing sequence, computes the moves that led from the first > to the second configuration. > > Signed-off-by: Klaus Aehlig <[email protected]> > --- > src/Ganeti/HTools/Cluster.hs | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/src/Ganeti/HTools/Cluster.hs b/src/Ganeti/HTools/Cluster.hs > index 6eb0ef1..c647221 100644 > --- a/src/Ganeti/HTools/Cluster.hs > +++ b/src/Ganeti/HTools/Cluster.hs > @@ -46,6 +46,7 @@ module Ganeti.HTools.Cluster > , printSolutionLine > , formatCmds > , involvedNodes > + , getMoves > , splitJobs > -- * Display functions > , printNodes > @@ -1412,6 +1413,21 @@ involvedNodes il plc = > inst = Container.find i il > in nub . filter (>= 0) $ [np, ns] ++ Instance.allNodes inst > > +-- | From two adjacent cluster tables get the list of moves that > transitions > +-- from to the other > +getMoves :: (Table, Table) -> [MoveJob] > +getMoves (Table _ iil _ iplc, Table fnl _ _ fplc) = > + let > + plctoMoves (plc@(idx, p, s, mv, _)) = > + let inst = Container.find idx iil > + inam = Instance.name inst > + afn = involvedNodes iil plc > + np = Node.alias $ Container.find p fnl > + ns = Node.alias $ Container.find s fnl > + (_, cmds) = computeMoves inst inam mv np ns > + in (afn, idx, mv, cmds) > + in map plctoMoves . reverse . drop (length iplc) $ reverse fplc > + > -- | Inner function for splitJobs, that either appends the next job to > -- the current jobset, or starts a new jobset. > mergeJobs :: ([JobSet], [Ndx]) -> MoveJob -> ([JobSet], [Ndx]) > -- > 1.8.4.1 > > Hrvoje Ribicic Ganeti Engineering Google Germany GmbH Dienerstr. 12, 80331, München Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Graham Law, Christine Elizabeth Flores Steuernummer: 48/725/00206 Umsatzsteueridentifikationsnummer: DE813741370
