Ori Liel has posted comments on this change.

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


Patch Set 5: (2 inline comments)

....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendResource.java
Line 423:         return getEntity(List.class,
Line 424:                 VdcQueryType.GetConsoleDevices,
Line 425:                 new IdQueryParameters(id),
Line 426:                 "GetConsoleDevices", true);
Line 427:     }
Even though the method is common to three difference resources, it can't go in 
here. The current heirarchy indeed makes it difficult to reuse this code. It 
would be ideal to have a utility object which all three refer to and use - but 
the way the API layer is built this is not feasible for several reasons 
(mainly, a whole 'state' would have to be injected into that object). 
Admittedly, there is some over-use of inheritance, as opposed to composition, 
in the API layer, which causes these types of problems. But given that this is 
the state of things, I would ask to to replicate this method three times.


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmPoolsResource.java
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());
Line 68: 
Added a comment to your conversation in PS3
Line 69:         return performCreate(VdcActionType.AddVmPoolWithVms,
Line 70:                                params,
Line 71:                                new 
QueryIdResolver<Guid>(VdcQueryType.GetVmPoolById,
Line 72:                                                    
IdQueryParameters.class));


-- 
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: 5
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