Re: [Engine-devel] LockManager maybe not be really taking locks in your flows...

2013-08-12 Thread Allon Mureinik


- Original Message -
 From: Roy Golan rgo...@redhat.com
 To: Yair Zaslavsky yzasl...@redhat.com
 Cc: engine-devel engine-devel@ovirt.org
 Sent: Sunday, August 11, 2013 5:16:47 PM
 Subject: Re: [Engine-devel] LockManager maybe not be really taking locks in 
 your flows...
 
 On Sun 11 Aug 2013 05:14:04 PM IDT, Roy Golan wrote:
  On Sun 11 Aug 2013 05:03:05 PM IDT, Yair Zaslavsky wrote:
  Hi all,
  Thanks to Alon Bar Lev's efforts for preventing to concurrent host
  installation for the same host entity + Roy Golan's check of the
  code, we saw that for commands that override
 
  getExclusiveLocks() or getSharedLocks() the locking mechanism does
  not work (lock is not being acquired) if there is no annotation of
  @LockIdNameAttribute on the class.
  A bug was filed for removing this annotation (leftover from some
  historical code ) , but until it is fixed - bare in mind you need to
  add this annotation in current commands you are working on
  in order to utilize the mechanism.
  See RemoveVmCommand (has the annotation) vs AddDiskCommand (which
  doesn't have the annotation)
 
 
  Cheers,
  Yair
  ___
  Engine-devel mailing list
  Engine-devel@ovirt.org
  http://lists.ovirt.org/mailman/listinfo/engine-devel
 
  just to make it clear, the storage commands are calling explicitly
  aquireLockInternal in the canDoAction. since there is no contract (no
  interface) we are open to mistakes and misuse. this should be rectified.
 
 more info from the bug:
 see the list of commands that overrrides getExcelusiveLocks but *don't*
 have the @LockIdNameAttribute annotation:
 
 bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
Manually calls acquireLockInternal() - disgusting, but works (on my todo list 
for some future version, don't worry).

 bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java
Same.

 bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java
 bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmTemplateCommand.java
 bll/src/main/java/org/ovirt/engine/core/bll/ExportRepoImageCommand.java
 
 ___
 Engine-devel mailing list
 Engine-devel@ovirt.org
 http://lists.ovirt.org/mailman/listinfo/engine-devel
 
___
Engine-devel mailing list
Engine-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel


Re: [Engine-devel] LockManager maybe not be really taking locks in your flows...

2013-08-12 Thread Allon Mureinik


- Original Message -
 From: Allon Mureinik amure...@redhat.com
 To: Roy Golan rgo...@redhat.com
 Cc: engine-devel engine-devel@ovirt.org
 Sent: Monday, August 12, 2013 9:59:17 AM
 Subject: Re: [Engine-devel] LockManager maybe not be really taking locks in 
 your flows...
 
 
 
 - Original Message -
  From: Roy Golan rgo...@redhat.com
  To: Yair Zaslavsky yzasl...@redhat.com
  Cc: engine-devel engine-devel@ovirt.org
  Sent: Sunday, August 11, 2013 5:16:47 PM
  Subject: Re: [Engine-devel] LockManager maybe not be really taking locks in
  your flows...
  
  On Sun 11 Aug 2013 05:14:04 PM IDT, Roy Golan wrote:
   On Sun 11 Aug 2013 05:03:05 PM IDT, Yair Zaslavsky wrote:
   Hi all,
   Thanks to Alon Bar Lev's efforts for preventing to concurrent host
   installation for the same host entity + Roy Golan's check of the
   code, we saw that for commands that override
  
   getExclusiveLocks() or getSharedLocks() the locking mechanism does
   not work (lock is not being acquired) if there is no annotation of
   @LockIdNameAttribute on the class.
   A bug was filed for removing this annotation (leftover from some
   historical code ) , but until it is fixed - bare in mind you need to
   add this annotation in current commands you are working on
   in order to utilize the mechanism.
   See RemoveVmCommand (has the annotation) vs AddDiskCommand (which
   doesn't have the annotation)
  
  
   Cheers,
   Yair
   ___
   Engine-devel mailing list
   Engine-devel@ovirt.org
   http://lists.ovirt.org/mailman/listinfo/engine-devel
  
   just to make it clear, the storage commands are calling explicitly
   aquireLockInternal in the canDoAction. since there is no contract (no
   interface) we are open to mistakes and misuse. this should be rectified.
  
  more info from the bug:
  see the list of commands that overrrides getExcelusiveLocks but *don't*
  have the @LockIdNameAttribute annotation:
  
  bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
 Manually calls acquireLockInternal() - disgusting, but works (on my todo list
 for some future version, don't worry).
 
  bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java
 Same.
 
  bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java
  bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmTemplateCommand.java

  bll/src/main/java/org/ovirt/engine/core/bll/ExportRepoImageCommand.java
Fixed: http://gerrit.ovirt.org/#/c/17946/

  
  ___
  Engine-devel mailing list
  Engine-devel@ovirt.org
  http://lists.ovirt.org/mailman/listinfo/engine-devel
  
 ___
 Engine-devel mailing list
 Engine-devel@ovirt.org
 http://lists.ovirt.org/mailman/listinfo/engine-devel
 
___
Engine-devel mailing list
Engine-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel


[Engine-devel] LockManager maybe not be really taking locks in your flows...

2013-08-11 Thread Yair Zaslavsky
Hi all,
Thanks to Alon Bar Lev's efforts for preventing to concurrent host installation 
for the same host entity + Roy Golan's check of the code, we saw that for 
commands that override

getExclusiveLocks() or getSharedLocks() the locking mechanism does not work 
(lock is not being acquired) if there is no annotation of @LockIdNameAttribute 
on the class.
A bug was filed for removing this annotation (leftover from some historical 
code ) , but until it is fixed - bare in mind you need to add this annotation 
in current commands you are working on 
in order to utilize the mechanism.
See RemoveVmCommand (has the annotation) vs AddDiskCommand (which doesn't have 
the annotation)


Cheers,
Yair
___
Engine-devel mailing list
Engine-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel


Re: [Engine-devel] LockManager maybe not be really taking locks in your flows...

2013-08-11 Thread Roy Golan

On Sun 11 Aug 2013 05:03:05 PM IDT, Yair Zaslavsky wrote:

Hi all,
Thanks to Alon Bar Lev's efforts for preventing to concurrent host installation 
for the same host entity + Roy Golan's check of the code, we saw that for 
commands that override

getExclusiveLocks() or getSharedLocks() the locking mechanism does not work 
(lock is not being acquired) if there is no annotation of @LockIdNameAttribute 
on the class.
A bug was filed for removing this annotation (leftover from some historical 
code ) , but until it is fixed - bare in mind you need to add this annotation 
in current commands you are working on
in order to utilize the mechanism.
See RemoveVmCommand (has the annotation) vs AddDiskCommand (which doesn't have 
the annotation)


Cheers,
Yair
___
Engine-devel mailing list
Engine-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel


just to make it clear, the storage commands are calling explicitly 
aquireLockInternal in the canDoAction. since there is no contract (no 
interface) we are open to mistakes and misuse. this should be rectified.


___
Engine-devel mailing list
Engine-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel


Re: [Engine-devel] LockManager maybe not be really taking locks in your flows...

2013-08-11 Thread Roy Golan

On Sun 11 Aug 2013 05:14:04 PM IDT, Roy Golan wrote:

On Sun 11 Aug 2013 05:03:05 PM IDT, Yair Zaslavsky wrote:

Hi all,
Thanks to Alon Bar Lev's efforts for preventing to concurrent host
installation for the same host entity + Roy Golan's check of the
code, we saw that for commands that override

getExclusiveLocks() or getSharedLocks() the locking mechanism does
not work (lock is not being acquired) if there is no annotation of
@LockIdNameAttribute on the class.
A bug was filed for removing this annotation (leftover from some
historical code ) , but until it is fixed - bare in mind you need to
add this annotation in current commands you are working on
in order to utilize the mechanism.
See RemoveVmCommand (has the annotation) vs AddDiskCommand (which
doesn't have the annotation)


Cheers,
Yair
___
Engine-devel mailing list
Engine-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel


just to make it clear, the storage commands are calling explicitly
aquireLockInternal in the canDoAction. since there is no contract (no
interface) we are open to mistakes and misuse. this should be rectified.


more info from the bug:
see the list of commands that overrrides getExcelusiveLocks but *don't* 
have the @LockIdNameAttribute annotation:


bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java
bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java
bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmTemplateCommand.java
bll/src/main/java/org/ovirt/engine/core/bll/ExportRepoImageCommand.java

___
Engine-devel mailing list
Engine-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel