Peter V. Saveliev has posted comments on this change. Change subject: RAM snapshots feature ......................................................................
Patch Set 1: I would prefer that you didn't submit this (2 inline comments) have just a minor issue. overall the patchset is ok for me. .................................................... File vdsm/API.py Line 636: v = self._cif.vmContainer.get(self._UUID) Line 637: if not v: Line 638: return errCode['noVM'] Line 639: memoryParams = {} Line 640: if snapMemory: (see BindingXMLRPC comment) «if snapMemory is not None» Line 641: memoryParams['dst'], memoryParams['dstparams'] = \ Line 642: self._getHibernationPaths(snapMemory) Line 643: return v.snapshot(snapDrives, memoryParams) Line 644: .................................................... 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=''): is it a good idea to provide here an empty string? I would rather use «None» here, since None clearly defines uninitialized variable. 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 <aha...@redhat.com> Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Eduardo <ewars...@redhat.com> Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com> Gerrit-Reviewer: Peter V. Saveliev <p...@redhat.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches