LGTM, thanks.

On Tue, Mar 4, 2014 at 1:25 PM, Hrvoje Ribicic <[email protected]> wrote:

> 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