Ori Liel has posted comments on this change.
Change subject: core: restapi: wip: introducing
ExportDiskBySnapshotAsBlockDevice
......................................................................
Patch Set 3: (5 inline comments)
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/SnapshotDiskResource.java
Line 13: import org.ovirt.engine.api.model.Disk;
Line 14:
Line 15: @Produces({ApiMediaType.APPLICATION_XML,
ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML})
Line 16: public interface SnapshotDiskResource {
Line 17:
Now that this resource has an 'action' that it can do, you need to also add the
method: getActionSubresource(...) you can take an example from any resource
which contains actions, for example, in VmResource:
@Path("{action:
(start|stop|shutdown|suspend|detach|migrate|export|move|ticket|cancelmigration)}/{oid}")
public ActionResource getActionSubresource(@PathParam("action")String
action, @PathParam("oid")String oid);
Line 18: @GET
Line 19: @Formatted
Line 20: public Disk get();
Line 21:
Line 21:
Line 22: @POST
Line 23: @Formatted
Line 24: @Actionable
Line 25: @Path("exportAsBlockDevice")
everything should be lowercase; names of all actions in the api - even if they
contain more than one word - are lowercase.
Line 26: @Consumes({ApiMediaType.APPLICATION_XML,
ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML})
Line 27: public Response exportAsBlockDevice(Action action);
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml
Line 653: parameterType: null
Line 654: signatures: []
Line 655: urlparams: {}
Line 656: headers: {}
Line 657: - name:
/api/vms/{vm:id}/snapshots/{snapshot:id}/disks/{disk:id}/exportAsBlockDevice|rel=get
a) exportAsBlockDevice should be lower-case (gave the same comment inside the
interface class)
b) 'rel' should be the name of the action: (change rel=get to
rel.exportasblockdevice)
Line 658: request:
Line 659: body:
Line 660: parameterType: null
Line 661: signatures: []
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendSnapshotDiskResource.java
Line 33:
Line 34: @Override
Line 35: public Response exportAsBlockDevice(Action action) {
Line 36: validateParameters(action, "host");
Line 37: validateParameters(action.getHost(), "id");
This syntax should work:
validateParameters(action, "host.id");
(instead of the two above lines).
Line 38: ExportDiskBySnapshotAsBlockDeviceParameters params = new
ExportDiskBySnapshotAsBlockDeviceParameters(Guid.createGuidFromString(diskId),
collection.parent.guid, Guid.createGuidFromString(action.getHost().getId()));
Line 39: return
doAction(VdcActionType.ExportDiskBySnapshotAsBlockDevice, params, action);
Line 40: }
Line 41:
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendSnapshotDisksResource.java
Line 9: import org.ovirt.engine.api.restapi.types.DiskMapper;
Line 10: import org.ovirt.engine.core.common.businessentities.DiskImage;
Line 11: import org.ovirt.engine.core.common.businessentities.VM;
Line 12:
Line 13: public class BackendSnapshotDisksResource extends
AbstractBackendCollectionResource<Disk,
org.ovirt.engine.core.common.businessentities.Disk> implements
SnapshotDisksResource {
I agree that BackendSnapshotElementsResource is superfluous. It was designed to
hold logic which is common to snapshot elements: CdRoms, Disks, Nics, but ended
up being empty and containing no logic.
But if you choose that BackendSnapshotDisksResource won't inherit it, you must
maintain consistency. BackendSnapshotCdRomsResource and
BackendSnapshotNicsResource also inherit it. You should make them inherit
AbstractBackendCollectionResource as well, then delete
BackendSnapshotElementsResource.
Line 14:
Line 15: protected BackendSnapshotResource parent;
Line 16:
Line 17: public BackendSnapshotDisksResource(BackendSnapshotResource
parent) {
--
To view, visit http://gerrit.ovirt.org/16284
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f96c2d8f4e0507383accfceff217e9719332607
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Deepak C Shetty <deepa...@linux.vnet.ibm.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Ori Liel <ol...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
