Omer Frenkel has posted comments on this change.

Change subject: core: enable migration to a different cluster
......................................................................


Patch Set 4:

(1 comment)

when changing vm cluster, there are many validation we need to do (look at 
ChangeVMClusterCommand) we need to find a nice way to run all these validation, 
or users will be able to get into a messy state

http://gerrit.ovirt.org/#/c/34316/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmToServerCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmToServerCommand.java:

Line 46:         }
Line 47: 
Line 48:         // the protection should be here - selecting a host from a 
different cluster should be enabled only if the user explicitly sets also the 
cluster
Line 49:         // so he actually knows what he is doing and does not select a 
host from a different cluster accidentally.
Line 50:         if (getVm().getRunOnVds() != null && 
getVm().getRunOnVds().equals(getDestinationVdsId()) && 
getParameters().getTargetVdsGroupId() == null) {
something here is wrong, this should have been added to the check on the 
cluster, not on this check (for same host)

also we might want to add a check that the selected host is actually on the 
selected cluster? (see general note on missing validations on the patch)
Line 51:             return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_MIGRATION_TO_SAME_HOST);
Line 52:         }
Line 53: 
Line 54:         return true;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9eb4fd7f10d3b0e63455a81f0b5c1389586a43f4
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[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