Juan Hernandez has posted comments on this change. Change subject: restapi: clone vm ......................................................................
Patch Set 1: (4 comments) Looks good, some minor comments inside. http://gerrit.ovirt.org/#/c/26526/1/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/VmResource.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/VmResource.java: Line 33: Line 34: @Produces({ApiMediaType.APPLICATION_XML, ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML}) Line 35: public interface VmResource extends UpdatableResource<VM>, AsynchronouslyCreatedResource, MeasurableResource { Line 36: Line 37: @Path("{action: (start|stop|shutdown|reboot|suspend|detach|migrate|export|move|ticket|cancelmigration|preview_snapshot|commit_snapshot|undo_snapshot|clone_vm)}/{oid}") I think that the name of this operation should be just "clone". Line 38: public ActionResource getActionSubresource(@PathParam("action")String action, @PathParam("oid")String oid); Line 39: Line 40: @POST Line 41: @Formatted Line 132: @POST Line 133: @Formatted Line 134: @Consumes({ApiMediaType.APPLICATION_XML, ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML}) Line 135: @Actionable Line 136: @Path("clone_vm") Please rename to just "clone". Line 137: public Response cloneVm(Action action); Line 138: Line 139: @POST Line 140: @Formatted http://gerrit.ovirt.org/#/c/26526/1/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 401: urlparams: {} Line 402: headers: Line 403: Content-Type: {value: application/xml|json, required: true} Line 404: Correlation-Id: {value: 'any string', required: false} Line 405: - name: /api/vms/{vm:id}/clone_vm|rel=clone_vm Please replace "clone_vm" with "clone". Line 406: request: Line 407: body: Line 408: parameterType: Action Line 409: signatures: http://gerrit.ovirt.org/#/c/26526/1/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 259: } Line 260: Line 261: @Override Line 262: public Response cloneVm(Action action) { Line 263: org.ovirt.engine.core.common.businessentities.VM vm = getEntity(org.ovirt.engine.core.common.businessentities.VM.class, VdcQueryType.GetVmByVmId, new IdQueryParameters(guid), "GetVmByVmId"); The purpose of the last parameter to the "getEntity" method is to build a useful error for the user, in case of failure. Usually it is something that indicates what was the identifier provided, something like this: "VM: id=" + guid Line 264: // in case not set let the server to handle it in canDoAction Line 265: String newName = ""; Line 266: if (action.isSetCloneConfiguration() && action.getCloneConfiguration().isSetName()) { Line 267: newName = action.getCloneConfiguration().getName(); -- 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: 1 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
