Michael Pasternak has posted comments on this change.
Change subject: restapi: Support Floating Disks
......................................................................
Patch Set 2: (9 inline comments)
....................................................
File
backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/util/LinkHelper.java
Line 193
no more usage for DeviceResource?
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/DiskResource.java
Line 16
why did you removed copyright notice?
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/VmDiskResource.java
Line 35: public interface VmDiskResource extends DeviceResource<Disk>,
MeasurableResource {
any reason for not inheriting from DiskResource & reusing it's signatures?
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata_v-3.1.yaml
Line 251: headers: {}
you should not add another delete signature, but add parameters overload to
the old delete(), i.e another parameters_set
Line 285: - mandatoryArguments: {disk.id: 'xs:string'}
how disk.id can be mandatory for all add() disk operations?,
you should add another parameters overload for /attach/
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDiskResource.java
Line 75: return super.get();//explicit call solves REST-Easy confusion
did you filed bug on this for rest-easy?
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResource.java
Line 63: if (action.isDetach()) {
potential NPE, check if <detach> was set before using it
Line 64: AttachDettachVmDiskParameters params = new
AttachDettachVmDiskParameters(parentId, Guid.createGuidFromString(id), true);
btw i'd use anonymous class definition, there is no need for variablre here
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/DiskStatisticalQuery.java
Line 43: disk.setVm(new VM());
disk.setVm(new VM()); should be inside of this /if/
--
To view, visit http://gerrit.ovirt.org/4665
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I314ba532b1a40286053f87bb4f0f3c90b5850b5a
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ori Liel <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Ori Liel <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
