Arik Hadas has posted comments on this change.

Change subject: core: [WIP] add memory snapshots capability to VdsBroker
......................................................................


Patch Set 7: (3 inline comments)

....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/SnapshotVDSCommandParameters.java
Line 5: import org.ovirt.engine.core.common.businessentities.DiskImage;
Line 6: import org.ovirt.engine.core.compat.Guid;
Line 7: 
Line 8: public class SnapshotVDSCommandParameters extends 
VdsAndVmIDVDSParametersBase {
Line 9:     /** Those are the disks images that snapshot should be taken for */
it's funny that there is no documentation almost anywhere in the system, and 
when such is added there are comment about it :) but ok - np
Line 10:     private List<DiskImage> images;
Line 11:     /** String representation of the volume in which the memory will 
be dump to */
Line 12:     private String memoryVolume;
Line 13: 


Line 15:         super(vdsId, vmId);
Line 16:         this.images = images;
Line 17:     }
Line 18: 
Line 19:     public SnapshotVDSCommandParameters(Guid vdsId, Guid vmId, 
List<DiskImage> images, String memoryVolume) {
I call both of them at 
CreateAllSnapshotsFromVmCommand#buildLiveSnapshotParameters, depending on 
whether the ram snapshots capability is supported or not. basically the 
memoryVolume parameter is optional and since there are no optional parameters 
in Java that's probably the best way to handle optional parameters (I don't 
like the caller to send 'default' value in that case, IMO it's just wrong - the 
default value should be known by the class itself, from OO perspective)
Line 20:         this(vdsId, vmId, images);
Line 21:         this.memoryVolume = memoryVolume;
Line 22:     }
Line 23: 


....................................................
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsServerWrapper.java
Line 843:         return vdsServer.futurePing();
Line 844:     }
Line 845: 
Line 846:     @Override
Line 847:     public StatusOnlyReturnForXmlRpc snapshot(String vmId, 
Map<String, String>[] disks) {
because we might work with vdsm that doesn't have the updated verb with 3 
parameters (in a < 3.3 cluster compatibility level)
Line 848:         try {
Line 849:             Map<String, Object> xmlRpcReturnValue = 
vdsServer.snapshot(vmId, disks);
Line 850:             StatusOnlyReturnForXmlRpc wrapper = new 
StatusOnlyReturnForXmlRpc(xmlRpcReturnValue);
Line 851:             return wrapper;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id928c76562d520acbf36ca01e074e51cbdcccaf9
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to