Ori Liel has posted comments on this change.

Change subject: restapi: Make adding (virtio-)console to virtual machines 
optional
......................................................................


Patch Set 3: (1 inline comment)

....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmPoolsResource.java
Line 63: 
Line 64:         int size = pool.isSetSize() ? pool.getSize() : 1;
Line 65: 
Line 66:         AddVmPoolWithVmsParameters params = new 
AddVmPoolWithVmsParameters(entity, vm, size, -1);
Line 67:         
params.setConsoleEnabled(!getConsoleDevicesForEntity(template.getId()).isEmpty());
I think this logic should move to the engine. 

It looks like console enabled/disabled is not the user's choice, but rather is 
directly inferred from the state of the vm-pool in the engine (according to 
whether this vm-pool have console devices or not)

So what happens is, you fetch information from the engine (using 
getConsoleDevicesForEntity) to decide what send back to the engine. Unless I'm 
missing something, this makes no sense: the engine can receive the request 
without consoleEnabled flag, and check internally whether or not the pool has 
console devices.
Line 68: 
Line 69:         return performCreate(VdcActionType.AddVmPoolWithVms,
Line 70:                                params,
Line 71:                                new 
QueryIdResolver<Guid>(VdcQueryType.GetVmPoolById,


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I733e4c4a9c7d8f5dbe68b6b26a49502b8a40ec9d
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Ori Liel <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to