Re: [PATCH master 3/5] Make hroller also plan for non-redundant instances

[Guido Trotter]

On Wed, May 22, 2013 at 10:50 AM, Klaus Aehlig <aeh...@google.com> wrote:

>
> > > +-- | Compute the result of moving an instance to a different node.
> > > +move :: Idx -> Ndx -> (Node.List, Instance.List)
> > > +        -> OpResult (Node.List, Instance.List)
> > > +move idx new_ndx (nl, il) = do
> > > +  let new_node = Container.find new_ndx nl
> > > +      inst = Container.find idx il
> > > +      old_ndx = Instance.pNode inst
> > > +      old_node = Container.find old_ndx nl
> > > +  new_node' <- Node.addPriEx True new_node inst
> > > +  let old_node' = Node.removePri old_node inst
> > > +      inst' = Instance.setPri inst new_ndx
> > > +      nl' = Container.addTwo old_ndx old_node' new_ndx new_node' nl
> > > +      il' = Container.add idx inst' il
> > > +  return (nl', il')
> > >
> >
> > Are we just "picking" a new node for the instance? What if it doesn't
> fit?
>
> The Node.addPriEx already verifies memory, disk, CPU, and tags (that's
> why it lives in the OpResult monad). In fact, as far as I can see, the
> only verification in hail when allocating a non-redundant instance is
> also only done by calling Node.addPri (a short form of Node.addPriEx).
>
> Am I missing something?
>
>
No, sorry, I was confused myself. LGTM then.

Thanks,

Guido