Vitor de Lima has posted comments on this change.
Change subject: core, webadmin, restapi: Fix VM creation with blank template
......................................................................
Patch Set 8:
(2 comments)
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java
Line 108:
Line 109: VDSGroup cluster =
lookupCluster(staticVm.getVdsGroupId());
Line 110: if (cluster.getArchitecture() ==
ArchitectureType.undefined) {
Line 111: throw new
WebApplicationException(Response.status(Response.Status.BAD_REQUEST)
Line 112: .entity("The selected cluster does not
have architecture, please set it before adding vms.")
What do you think about dropping this validation and for the cases where the
target cluster have an undefined architecture we let the canDo of the
AddVmCommand fail properly? (This will happen due to the recent merge of the
change #18227)
Line 113: .build());
Line 114: }
Line 115:
Line 116: if (Guid.Empty.equals(templateId) && !vm.isSetOs()) {
Line 155:
Line 156: if (defaultOs == null) {
Line 157: throw new
WebApplicationException(Response.status(Response.Status.BAD_REQUEST)
Line 158: .entity("Could not determine the default OS based
on the architecture of the selected cluster.")
Line 159: .build());
I thought about replacing this method with:
private void replaceDefaultOs(VmStatic vm, VDSGroup cluster)
{
OsRepository osRepository =
SimpleDependecyInjector.getInstance().get(OsRepository.class);
if (cluster.getArchitecture() != ArchitectureType.undefined) {
Integer defaultOs =
osRepository.getDefaultOSes().get(cluster.getArchitecture());
vm.setOsId(defaultOs);
}
}
Because it doesn't matter which OS we pass to the command, it will fail anyway
if the architecture is undefined.
Line 160: }
Line 161:
Line 162: vm.setOsId(defaultOs);
Line 163: }
--
To view, visit http://gerrit.ovirt.org/20667
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I880e5940c7244476e477e098445a47210ea08b5d
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vitor de Lima <[email protected]>
Gerrit-Reviewer: Gustavo Frederico Temple Pedrosa
<[email protected]>
Gerrit-Reviewer: Leonardo Bianconi <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Vitor de Lima <[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