Juan Hernandez has posted comments on this change.

Change subject: restapi: clone vm
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.ovirt.org/#/c/26526/4/backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml
File 
backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml:

Line 406:   request:
Line 407:     body:
Line 408:       parameterType: Action
Line 409:       signatures:
Line 410:       - mandatoryArguments: {action.vm.name}
This is missing the type, should be:

  {action.vm.name: 'xs:string'}

Is this parameter really mandatory? I see in the implementation of the 
"cloneVm" method that we aren't checking the presence of the parameter, so I 
think it should be inside "optionalArguments" instead of "mandatoryArguments".

If it is mandatory, then in the "cloneVm" method we should do this first thing:

  validateParameters(action, "vm.name");
Line 411:     urlparams: {}
Line 412:     headers:
Line 413:       Content-Type: {value: application/xml|json, required: true}
Line 414:       Correlation-Id: {value: 'any string', required: false}


http://gerrit.ovirt.org/#/c/26526/4/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java:

Line 261:         return response;
Line 262:     }
Line 263: 
Line 264:     @Override
Line 265:     public Response cloneVm(Action action) {
If the vm.name parameter is mandatory then we should add here the following:

  validateParameters(action, "vm.name");

But I understand from the code below that it ins't really mandatory, so we 
should fix the rsdl_metatata.yaml file instead.
Line 266:         org.ovirt.engine.core.common.businessentities.VM vm = 
getEntity(
Line 267:                 
org.ovirt.engine.core.common.businessentities.VM.class,
Line 268:                 VdcQueryType.GetVmByVmId,
Line 269:                 new IdQueryParameters(guid), "VM: id=" + guid);


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I18cb01dc83f0a57a7a572fa0da0c7e3397c43707
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <tjeli...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com>
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