Michael Pasternak has posted comments on this change.

Change subject: restapi: Start VM not as RunOnce
......................................................................


Patch Set 1: Code-Review-1

(3 comments)

....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java
Line 226:             RunVmOnceParams params =
Line 227:                     map(map(getEntity(entityType, 
VdcQueryType.GetVmByVmId, new IdQueryParameters(guid), id, true),
Line 228:                             new VM()),
Line 229:                             new RunVmOnceParams(guid));
Line 230:             if (action.isSetVm()) {
this condition is no longer relevant
Line 231:                 validateEnums(VM.class, action.getVm());
Line 232:                 VM vm = action.getVm();
Line 233:                 params = map(vm, params);
Line 234:                 if (vm.isSetPlacementPolicy() && 
vm.getPlacementPolicy().isSetHost()) {


Line 228:                             new VM()),
Line 229:                             new RunVmOnceParams(guid));
Line 230:             if (action.isSetVm()) {
Line 231:                 validateEnums(VM.class, action.getVm());
Line 232:                 VM vm = action.getVm();
not exactly related to this change, but worth fixing in this context:

you can save mapping round trip if you'll pass this 'vm' object as a template 
to 

mapping took place at L225 instead of 'new VM()' and doping map() at L233
Line 233:                 params = map(vm, params);
Line 234:                 if (vm.isSetPlacementPolicy() && 
vm.getPlacementPolicy().isSetHost()) {
Line 235:                     validateParameters(vm.getPlacementPolicy(), 
"host.id|name");
Line 236:                     
params.setDestinationVdsId(getHostId(vm.getPlacementPolicy().getHost()));


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmResourceTest.java
Line 384:     }
Line 385: 
Line 386:     @Test
Line 387:     public void testStart() throws Exception {
Line 388:         setUriInfo(setUpActionExpectations(VdcActionType.RunVm,
please instead of changing test from RunVmOnce to RunVm, adapt RunVmOnce test 
to your change and add new test for RunVm,

thanks.
Line 389:                                            RunVmParams.class,
Line 390:                                            new String[] { "VmId" },
Line 391:                                            new Object[] { GUIDS[0] 
}));
Line 392: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40be200c8742d8b31b7881926c59deaf345992ec
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Betak <[email protected]>
Gerrit-Reviewer: Martin Betak <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Ori Liel <[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