LGTM, thanks!

On Tue, Mar 4, 2014 at 1:18 PM, Klaus Aehlig <[email protected]> wrote:

> On Tue, Mar 04, 2014 at 11:26:43AM +0100, Hrvoje Ribicic wrote:
> > On Tue, Mar 4, 2014 at 11:23 AM, Petr Pudlák <[email protected]> wrote:
> >
> > > It seems that node-alloc/NAL should be a sublock of
> node-alloc/[lockset],
> > > but let's wait for Riba's comment.
> > >
> > > Otherwise LGTM.
> > >
> > >
> > +1 for this. It seems that inverting the direction of this relation would
> > not matter, and it would make the code more consistent.
>
> It also changes the lock order, i.e., you have to allocate
> node-alloc/[lockset]
> before node-alloc/NAL.
>
>
> commit ffb8671ab332e5c5e7872df5a8473778d0434699
> Author: Klaus Aehlig <[email protected]>
> Date:   Tue Mar 4 13:14:01 2014 +0100
>
>     Interdiff [PATCH master 2/5] Add Ganeti Lock Hierarchy
>
> diff --git a/src/Ganeti/Locking/Locks.hs b/src/Ganeti/Locking/Locks.hs
> index c0c078f..5dc8949 100644
> --- a/src/Ganeti/Locking/Locks.hs
> +++ b/src/Ganeti/Locking/Locks.hs
> @@ -51,8 +51,8 @@ data GanetiLocks = BGL
>                   | Instance String
>                   | NodeGroupLockSet
>                   | NodeGroup String
> -                 | NAL
>                   | NodeAllocLockSet
> +                 | NAL
>                   | NodeResLockSet
>                   | NodeRes String
>                   | NodeLockSet
> @@ -67,8 +67,8 @@ lockName InstanceLockSet = "instance/[lockset]"
>  lockName (Instance uuid) = "instance/" ++ uuid
>  lockName NodeGroupLockSet = "nodegroup/[lockset]"
>  lockName (NodeGroup uuid) = "nodegroup/" ++ uuid
> -lockName NAL = "node-alloc/NAL"
>  lockName NodeAllocLockSet = "node-alloc/[lockset]"
> +lockName NAL = "node-alloc/NAL"
>  lockName NodeResLockSet = "node-res/[lockset]"
>  lockName (NodeRes uuid) = "node-res/" ++ uuid
>  lockName NodeLockSet = "node/[lockset]"
> @@ -82,8 +82,8 @@ lockFromName "instance/[lockset]" = return
> InstanceLockSet
>  lockFromName (stripPrefix "instance/" -> Just uuid) = return $ Instance
> uuid
>  lockFromName "nodegroup/[lockset]" = return NodeGroupLockSet
>  lockFromName (stripPrefix "nodegroup/" -> Just uuid) = return $ NodeGroup
> uuid
> -lockFromName "node-alloc/NAL" = return NAL
>  lockFromName "node-alloc/[lockset]" = return NodeAllocLockSet
> +lockFromName "node-alloc/NAL" = return NAL
>  lockFromName "node-res/[lockset]" = return NodeResLockSet
>  lockFromName (stripPrefix "node-res/" -> Just uuid) = return $ NodeRes
> uuid
>  lockFromName "node/[lockset]" = return NodeLockSet
> @@ -99,7 +99,7 @@ instance Lock GanetiLocks where
>    lockImplications BGL = []
>    lockImplications (Instance _) = [InstanceLockSet, BGL]
>    lockImplications (NodeGroup _) = [NodeGroupLockSet, BGL]
> -  lockImplications NodeAllocLockSet = [NAL, BGL]
> +  lockImplications NAL = [NodeAllocLockSet, BGL]
>    lockImplications (NodeRes _) = [NodeResLockSet, BGL]
>    lockImplications (Node _) = [NodeLockSet, BGL]
>    lockImplications _ = [BGL]
>
>
> --
> 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