Liron Ar has posted comments on this change.

Change subject: core: add the domain detaching state
......................................................................


Patch Set 8:

(2 comments)

http://gerrit.ovirt.org/#/c/23556/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DetachStorageDomainFromPoolCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DetachStorageDomainFromPoolCommand.java:

Line 101:         addCanDoActionMessage(VdcBllMessages.VAR__ACTION__DETACH);
Line 102:     }
Line 103: 
Line 104:     @Override
Line 105:     protected Map<String, Pair<String, String>> getExclusiveLocks() {
1. this will cause to an issue, in RemoveStorageDomainCommand we take the lock 
by the domain id and than the detach command is being executed from there, as 
the lock is already taken we'll fail in the execution.

2. regardless , I'd have this lock added  in a separate patch but whatever you 
prefer.
Line 106:         return 
Collections.singletonMap(getParameters().getStorageDomainId().toString(),
Line 107:                 
LockMessagesMatchUtil.makeLockingPair(LockingGroup.STORAGE,
Line 108:                         
VdcBllMessages.ACTION_TYPE_FAILED_OBJECT_LOCKED));
Line 109:     }


http://gerrit.ovirt.org/#/c/23556/8/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/VdcActionUtils.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/VdcActionUtils.java:

Line 302:                 StorageDomainStatus.PreparingForMaintenance,
Line 303:                 EnumSet.of(VdcActionType.DetachStorageDomainFromPool, 
VdcActionType.DeactivateStorageDomain));
Line 304:         storageDomainMatrix.put(
Line 305:                 StorageDomainStatus.Detaching,
Line 306:                 EnumSet.of(VdcActionType.DeactivateStorageDomain, 
VdcActionType.ActivateStorageDomain));
don't we need to add here also removestoragedomain /detachstoragedomain/edit 
storage domain (extend)?
Line 307:         _matrix.put(StorageDomain.class, storageDomainMatrix);
Line 308:     }
Line 309: 
Line 310:     public static boolean canExecute(List<? extends 
BusinessEntityWithStatus<?, ?>> entities,


-- 
To view, visit http://gerrit.ovirt.org/23556
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5732ef00a67ef1381ee0b6f29d08ab39cf63a1bf
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to