Change in vdsm[master]: RAM snapshots feature

peet at redhat.com peet at redhat.com
Mon May 27 11:52:37 UTC 2013


Peter V. Saveliev has posted comments on this change.

Change subject: RAM snapshots feature
......................................................................


Patch Set 1: (1 inline comment)

....................................................
File vdsm/BindingXMLRPC.py
Line 279:     def vmUpdateDevice(self, vmId, params):
Line 280:         vm = API.VM(vmId)
Line 281:         return vm.vmUpdateDevice(params)
Line 282: 
Line 283:     def vmSnapshot(self, vmId, snapDrives, snapMemory=''):
I thinks that it is not clear from the code, that the engine uses empty strings for snapshots w/o memory. If someone will change snapMemory default to «None» in the future, the call will continue to work, but occasionally we can fail with the logic change: it is enough to start to use type check to get into error with cluster xor mem-less snapshots. And the cause of the fault will be not clear.

So ok, let us use an empty string by default, but please, write some detailed comment in the code about why there is an empty string (not «None» or «False»), and how behave all three cases you described above. I'm sure, it can be important in the future.
Line 284:         vm = API.VM(vmId)
Line 285:         return vm.snapshot(snapDrives, snapMemory)
Line 286: 
Line 287:     def vmMerge(self, vmId, mergeDrives):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I62401940afb0228cbd9dd3611b6ed8e0ff67c82c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <ahadas at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Arik Hadas <ahadas at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Peter V. Saveliev <peet at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list