Bug 997376

Summary: [command] remove the use of @LockIdNameAttribute
Product: Red Hat Enterprise Virtualization Manager Reporter: Yair Zaslavsky <yzaslavs>
Component: ovirt-engineAssignee: Yair Zaslavsky <yzaslavs>
Status: CLOSED DUPLICATE QA Contact:
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 3.3.0CC: acathrow, alonbl, bazulay, emesika, iheim, lpeer, oourfali, pstehlik, rgolan, Rhev-m-bugs, yeylon
Target Milestone: ---Keywords: CodeChange, Tracking, Triaged
Target Release: 3.4.0   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard: infra
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 995836 Environment:
Last Closed: 2013-11-08 08:28:55 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: Infra RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 995836    
Bug Blocks:    

Description Yair Zaslavsky 2013-08-15 10:50:07 UTC
+++ This bug was initially created as a clone of Bug #995836 +++

Annotations should use behavior modification of reference objects, not as controller of logic paths witin inheritance.

@LockIdNameAttribute annotation is used to indicate incomplete logic requirement.

Each command inherit from CommandBase, and should override the getExclusiveLocks() method to acquire lock.

there is no point in having annotation for the CommandBase to call the getExclusiveLocks() or any other optional feature, as it can have its own embedded default implementations.

--- Additional comment from Yair Zaslavsky on 2013-08-11 09:14:31 EDT ---

I would like to add that
annotations in java are not inherited .
Since we use inherience in the commands, this is a problem.

The problem exists in other commands - for example in AddDiskCommand.

--- Additional comment from Yair Zaslavsky on 2013-08-11 09:18:48 EDT ---

Bare in mind that although the keywords, we have issues with current behavior of several commands.
This is why I acked this to 3.3.

--- Additional comment from Roy Golan on 2013-08-11 10:04:49 EDT ---


grep "getExclusiveLocks" and extract what is missing @LockIdNameAttribute


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/RunVmCommandBase.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

--- Additional comment from Roy Golan on 2013-08-11 10:12:35 EDT ---

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.

--- Additional comment from Yair Zaslavsky on 2013-08-12 08:22:09 EDT ---

In reply to comment #3 -
Calling aquireLockInternal in CDA can still be risky, IMHO.
If we decide to postpone this from 3.3 - we should fix the above commands.

Comment 1 Yair Zaslavsky 2013-08-15 11:06:23 UTC
This was cloned for 3.4 - the infrastructure change will be held in this version.

Comment 2 Eli Mesika 2013-11-08 08:28:55 UTC

*** This bug has been marked as a duplicate of bug 995836 ***