Michael Kublin has posted comments on this change.

Change subject: core: Validate maximum number of hosts in DC.(#771699)
......................................................................


Patch Set 3: (3 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeVDSClusterCommand.java
Line 126:             }
Line 127:         });
Line 128: 
Line 129:         VdsSpmIdParameters vdsSpmIdParameters = new 
VdsSpmIdParameters();
Line 130:         vdsSpmIdParameters.setCompensationEnabled(true);
These is default value TransactionScopeOption.RequiresNew
Line 131:         
vdsSpmIdParameters.setTransactionScopeOption(TransactionScopeOption.RequiresNew);
Line 132:         vdsSpmIdParameters.setVdsId(getVds().getId());
Line 133: 
Line 134:         if (targetStoragePool != null


Line 161: 
Line 162:         if (getSourceCluster().getstorage_pool_id() != null
Line 163:                 && (targetStoragePool== null || 
!getSourceCluster().getstorage_pool_id().getValue().equals(targetStoragePool.getId())))
 {
Line 164:             
vdsSpmIdParameters.setStoragePoolId(getSourceCluster().getstorage_pool_id().getValue());
Line 165:             VdcReturnValueBase removeVdsSpmIdReturn =
These can not fail, if it will fail - we lost connection to DB, everything will 
fail.
Also I think for that flow we don't need VdcActionType.RemoveVdsSpmId and 
VdcActionType.AddVdsSpmId commands and we will have a very simple and good 
solution
Line 166:                     
Backend.getInstance().runInternalAction(VdcActionType.RemoveVdsSpmId,
Line 167:                             vdsSpmIdParameters,
Line 168:                             new 
CommandContext(getCompensationContext()));
Line 169:             if (!removeVdsSpmIdReturn.getSucceeded()) {


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsGroupCommand.java
Line 58:                 && 
!oldGroup.getstorage_pool_id().equals(getVdsGroup().getstorage_pool_id())
Line 59:                 || oldGroup.getstorage_pool_id() == null
Line 60:                 && getVdsGroup().getstorage_pool_id() != null) {
Line 61:             for (VdsStatic vds : 
getVdsStaticDAO().getAllForVdsGroup(oldGroup.getId())) {
Line 62:                 VdsSpmIdParameters parameters = new 
VdsSpmIdParameters(vds.getId(), getVdsGroup().getstorage_pool_id().getValue());
Maybe better, to add all spm_ids and after that remove all?
Line 63:                 if (getVdsGroup().getstorage_pool_id() != null) {
Line 64:                     VdcReturnValueBase addVdsSpmIdReturn =
Line 65:                             
getBackend().runInternalAction(VdcActionType.AddVdsSpmId, parameters);
Line 66:                     if (!addVdsSpmIdReturn.getSucceeded()) {


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If375400f3e12e3e0452053dea12ac6e28bc0ff61
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: Vered Volansky <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to