On Wed, Mar 07, 2012 at 03:07:00PM +0000, Guido Trotter wrote: > On Tue, Mar 6, 2012 at 11:59 PM, Iustin Pop <[email protected]> wrote: > > This brings together all previous pieces and allows balancing to work > > (with the known caveats related to disk space calculation) for shared > > storage disk templates. > > --- > > htools/Ganeti/HTools/Cluster.hs | 20 +++++++++++++++++++- > > htools/Ganeti/HTools/Types.hs | 2 ++ > > 2 files changed, 21 insertions(+), 1 deletions(-) > > > > diff --git a/htools/Ganeti/HTools/Cluster.hs > > b/htools/Ganeti/HTools/Cluster.hs > > index e2ec8b8..7c225f7 100644 > > --- a/htools/Ganeti/HTools/Cluster.hs > > +++ b/htools/Ganeti/HTools/Cluster.hs > > @@ -391,6 +391,17 @@ applyMove nl inst Failover = > > new_inst, old_sdx, old_pdx) > > in new_nl > > > > +-- Failover to any (fa) > > +applyMove nl inst (FailoverToAny new_pdx) = > > + let (old_pdx, old_sdx, old_p, _) = instanceNodes nl inst > > + tgt_n = Container.find new_pdx nl > > + !new_nl = do -- Maybe monad > > + new_p <- Node.addPriEx (Node.offline old_p) tgt_n inst > > + let new_inst = Instance.setPri inst new_pdx > > + return (Container.addTwo old_pdx (Node.removePri old_p inst) > > + new_pdx new_p nl, new_inst, new_pdx, old_sdx) > > + in new_nl > > + > > Can we have a bit more verbose variable names and explanations here?
Sure. I initially copied the code from the ReplaceSecondary branch, hence these names. The following interdiff attempts to improve the patch, while keeping the same overall naming as the rest of the applyMove cases (and in general with this file, e.g. nl = node list, pdx/sdx = pri/sec node index, etc.). Let me know what you think: diff --git a/htools/Ganeti/HTools/Cluster.hs b/htools/Ganeti/HTools/Cluster.hs index 835287b..1f868f5 100644 --- a/htools/Ganeti/HTools/Cluster.hs +++ b/htools/Ganeti/HTools/Cluster.hs @@ -396,15 +396,15 @@ applyMove nl inst Failover = in new_nl -- Failover to any (fa) -applyMove nl inst (FailoverToAny new_pdx) = - let (old_pdx, old_sdx, old_p, _) = instanceNodes nl inst - tgt_n = Container.find new_pdx nl - !new_nl = do -- Maybe monad - new_p <- Node.addPriEx (Node.offline old_p) tgt_n inst - let new_inst = Instance.setPri inst new_pdx - return (Container.addTwo old_pdx (Node.removePri old_p inst) - new_pdx new_p nl, new_inst, new_pdx, old_sdx) - in new_nl +applyMove nl inst (FailoverToAny new_pdx) = do + let (old_pdx, old_sdx, old_pnode, _) = instanceNodes nl inst + new_pnode = Container.find new_pdx nl + force_failover = Node.offline old_pnode + new_pnode' <- Node.addPriEx force_failover new_pnode inst + let old_pnode' = Node.removePri old_pnode inst + inst' = Instance.setPri inst new_pdx + nl' = Container.addTwo old_pdx old_pnode' new_pdx new_pnode' nl + return (nl', inst', new_pdx, old_sdx) -- Replace the primary (f:, r:np, f) applyMove nl inst (ReplacePrimary new_pdx) = -- iustin
