Omer Frenkel has posted comments on this change.

Change subject: core: adjustments needed to support instance types
......................................................................


Patch Set 16:

(2 comments)

http://gerrit.ovirt.org/#/c/23828/16/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CancelMigrateVmCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CancelMigrateVmCommand.java:

Line 33:     }
Line 34: 
Line 35:     @Override
Line 36:     protected boolean canDoAction() {
Line 37:         if (!super.canDoAction()) {
i dont understand why this is needed, we know the cluster(vds group) of the vm 
is not null, this command (and others that follow) are not touching the cluster 
(i also wrote this on change #12 and you agreed)
Line 38:             return false;
Line 39:         }
Line 40: 
Line 41:         if (getVm() == null) {


http://gerrit.ovirt.org/#/c/23828/16/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmCommand.java:

Line 487:     @Override
Line 488:     protected boolean canDoAction() {
Line 489:         if (getVdsGroup() == null) {
Line 490:             
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_CLUSTER_CAN_NOT_BE_EMPTY);
Line 491:             return false;
please use failCanDoAction method, its nicer
Line 492:         }
Line 493: 
Line 494:         return super.canDoAction();
Line 495:     }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifaebd849061efa1637f9fa21d8584212ba0e51f2
Gerrit-PatchSet: 16
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[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