Juan Hernandez has posted comments on this change.

Change subject: core, engine, restapi: Proper default migration policies for 
ppc64
......................................................................


Patch Set 4:

(2 comments)

....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendClustersResource.java
Line 83:         VDSGroup entity = map(cluster, map(pool));
Line 84: 
Line 85:         if (!cluster.isSetErrorHandling() || 
!cluster.getErrorHandling().isSetOnError()) {
Line 86:             entity.setMigrateOnError(null);
Line 87:         }
I think that this modification should be done in ClusterMapper.map(..) so it 
applies in all the places where a cluster is mapped. Currently it is as follows:

  if (model.isSetErrorHandling() && model.getErrorHandling().isSetOnError()) {
    entity.setMigrateOnError(map(model.getErrorHandling().getOnError(), null));
  }

So it won't change the property of the entity even if the error handling isn't 
specified. It should be changed to assign null in that case.
Line 88: 
Line 89:         return performCreate(VdcActionType.AddVdsGroup,
Line 90:                 new VdsGroupOperationParameters(entity),
Line 91:                 new 
QueryIdResolver<Guid>(VdcQueryType.GetVdsGroupById, IdQueryParameters.class));


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java
Line 119:                 }
Line 120: 
Line 121:                 if (!vm.isSetPlacementPolicy() && 
templateId.equals(Guid.Empty)) {
Line 122:                     staticVm.setMigrationSupport(null);
Line 123:                 }
I don't understand this rule. 
would you be so kind to explain? If it is business logic it should go in the 
backend.
Line 124: 
Line 125:                 Guid storageDomainId =
Line 126:                         (vm.isSetStorageDomain() && 
vm.getStorageDomain().isSetId()) ? asGuid(vm.getStorageDomain()
Line 127:                                 .getId())


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib34c4a01fe0c667bafc47bf70b33bb2990ffb7d3
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vitor de Lima <vitor.l...@eldorado.org.br>
Gerrit-Reviewer: Gustavo Frederico Temple Pedrosa 
<gustavo.pedr...@eldorado.org.br>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Leonardo Bianconi <leonardo.bianc...@eldorado.org.br>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Vitor de Lima <vitor.l...@eldorado.org.br>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to