Change in vdsm[master]: RAM snapshots feature

danken at redhat.com danken at redhat.com
Fri Jun 14 03:54:19 UTC 2013


Dan Kenigsberg has posted comments on this change.

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


Patch Set 6: I would prefer that you didn't submit this

(3 inline comments)

(A very partial review)

....................................................
File vdsm/hooks.py
Line 188: 
Line 189: 
Line 190: def before_vm_dehibernate(domxml, vmconf={}, params={}):
Line 191:     return _runHooksDir(domxml, 'before_vm_dehibernate', vmconf=vmconf,
Line 192:                         params=params)
Please document the new params in the man page.
Line 193: 
Line 194: 
Line 195: def after_vm_dehibernate(domxml, vmconf={}, params={}):
Line 196:     return _runHooksDir(domxml, 'after_vm_hibernate', vmconf=vmconf,


....................................................
File vdsm/vm.py
Line 2859:             self.setDownStatus(ERROR, 'failed to start libvirt vm')
Line 2860:             return
Line 2861:         self._domDependentInit()
Line 2862: 
Line 2863:     def _correctDiskVolumes(self, srcDomXML):
I think that "update" is a better term.
Line 2864:         """
Line 2865:         Update each image in the given XML to point to the right volume.
Line 2866:         Each image should have a newer volume than the one that was the
Line 2867:         newer when the snapshot was taken, since we create new volume


Line 2870:         for vmDrive in self._devices[DISK_DEVICES]:
Line 2871:             if vmDrive.device == 'disk':
Line 2872:                 pathToImageDir = os.path.dirname(vmDrive.path)
Line 2873:                 pattern = '(' + pathToImageDir + '.*)\''
Line 2874:                 mo = re.search(pattern, srcDomXML)
Doing a blind re.replace on an XML is a bug waiting to happen. The XML has structure and it is safer to use it. Please parse the XML and replace only the element you expect. Unless, of course, you have a particular intention here.
Line 2875:                 if mo:
Line 2876:                     srcDomXML = srcDomXML.replace(mo.group(1), vmDrive.path)
Line 2877:         return srcDomXML
Line 2878: 


-- 
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: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <ahadas 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