Liron Ar has posted comments on this change.

Change subject: core, restapi: Introducing support for attaching disk snapshot
......................................................................


Patch Set 31:

(4 comments)

....................................................
File 
backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/util/LinkHelper.java
Line 693:     public static <R extends BaseResource> R addLinks(UriInfo 
uriInfo, R model, Class<? extends BaseResource> suggestedParentType, boolean 
addActions) {
Line 694:         setHref(uriInfo, model, suggestedParentType);
Line 695:         if (addActions) {
Line 696:             setActions(uriInfo, model, suggestedParentType);
Line 697:         }
as discussed f2f, it's needed to avoid adding the actions in case we call the 
addLinks method "directly" on the parent , snapshot in that case(in that case, 
could be for any object)
Line 698: 
Line 699:         for (BaseResource inline : getInlineResources(model)) {
Line 700:             if (inline.getId() != null) {
Line 701:                 setHref(uriInfo, inline);


....................................................
File 
backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml
Line 534:           disk.size: xs:int #deprecated, replaced by 
'provisioned_size'
Line 535:           disk.sparse: xs:boolean
Line 536:           disk.bootable: xs:boolean
Line 537:           disk.shareable: xs:boolean
Line 538:           disk.snapshot.id: xs:string
Done
Line 539:           disk.propagate_errors: xs:boolean
Line 540:           disk.wipe_after_delete: xs:boolean
Line 541:           disk.storage_domains.storage_domain--COLLECTION: 
{storage_domain.id|name: 'xs:string'}
Line 542:         description: add a new disk to the virtual machine allocating 
space from the storage domain


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResource.java
Line 124:                             new 
IdQueryParameters(Guid.createGuidFromString(snapshotInfo.getId())));
Line 125:             if (queryReturnValue.getSucceeded() && 
queryReturnValue.getReturnValue() != null) {
Line 126:                 
org.ovirt.engine.core.common.businessentities.Snapshot snapshot = 
queryReturnValue.getReturnValue();
Line 127:                 VM vm = new VM();
Line 128:                 vm.setId(snapshot.getVmId().toString());
When querying the disk object, it has property with the guid of it's 
originating snapshot and not the complete snapshot object, thus we don't have 
the vm id. At the moment the backend won't fetch the snapshot info for each 
disk and we'll keep it as guid..therefore executing this is mandatory - when 
we'll possibly add other uses for this "snapshot" a query and related changes 
might be added/modified.
Line 129:                 snapshotInfo.setVm(vm);
Line 130:                 model.setSnapshot(snapshotInfo);
Line 131:             }
Line 132:             LinkHelper.addLinks(getUriInfo(), snapshotInfo, null, 
false);


....................................................
File 
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/DiskMapper.java
Line 112:         if (disk.isSetStatus()) {
Line 113:             
diskImage.setImageStatus(map(DiskStatus.fromValue(disk.getStatus().getState())));
Line 114:         }
Line 115:         if (disk.isSetSnapshot() && disk.getSnapshot().isSetId()) {
Line 116:             
diskImage.setVmSnapshotId(GuidUtils.asGuid(disk.getSnapshot().getId()));
Weird, i remembered adding that. Done (to the verify() method as the id is 
already populated).
Line 117:         }
Line 118:         if (disk.isSetSparse()) {
Line 119:             diskImage.setVolumeType(disk.isSparse() ? 
VolumeType.Sparse : VolumeType.Preallocated);
Line 120:         }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I02579bf1a91cd294a5040acf432f1fdb87eb18c1
Gerrit-PatchSet: 31
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Tal Nisan <[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