Francesco Romani has uploaded a new change for review. Change subject: virt: move handling of recovery file in a class ......................................................................
virt: move handling of recovery file in a class saveState() is little more than a fancy wrapper around pickle, tailored for the Vm special case. Extract all the saveState() related code in a small helper class, and move it in to recovery.py, to make the code easier to read and test. Change-Id: I6866eb2c3c9d5784c2054c860cbf0977e8e58031 Signed-off-by: Francesco Romani <[email protected]> --- M vdsm/virt/recovery.py M vdsm/virt/vm.py 2 files changed, 75 insertions(+), 55 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/88/51388/1 diff --git a/vdsm/virt/recovery.py b/vdsm/virt/recovery.py index ff33f8d..0e0af0e 100644 --- a/vdsm/virt/recovery.py +++ b/vdsm/virt/recovery.py @@ -20,6 +20,8 @@ import os import os.path +import tempfile +import threading import time import libvirt @@ -30,6 +32,72 @@ from vdsm import utils from .vm import getVDSMDomains +from . import vmstatus + + +class VmState(object): + """ + "pickle" for vm state. + """ + + _log = logging.getLogger("virt.recovery.vmstate") + + def __init__(self, vmid): + self._vmid = vmid + self._recovery_file = os.path.join( + constants.P_VDSM_RUN, + '%s.recovery' % vmid + ) + self._lock = threading.Lock() + + def cleanup(self): + with self._lock: + utils.rmFile(self._recovery_file) + self._recovery_file = None + + def save(self, vm): + data = self._collect(vm) + with self._lock: + if self._recovery_file is not None: + self._dump(data) + else: + self.log.debug('save after cleanup') + + def _dump(self, data): + with tempfile.NamedTemporaryFile( + dir=constants.P_VDSM_RUN, + delete=False + ) as f: + pickle.dump(data, f) + + os.rename(f.name, self._recovery_file) + + def _collect(self, vm): + data = vm.status() + data['startTime'] = vm.start_time + if vm.lastStatus != vmstatus.DOWN: + guestInfo = vm.guestAgent.getGuestInfo() + data['username'] = guestInfo['username'] + data['guestIPs'] = guestInfo['guestIPs'] + data['guestFQDN'] = guestInfo['guestFQDN'] + else: + data['username'] = "" + data['guestIPs'] = "" + data['guestFQDN'] = "" + if 'sysprepInf' in data: + del data['sysprepInf'] + if 'floppy' in data: + del data['floppy'] + for drive in data.get('drives', []): + for d in vm.getDiskDevices(): + if isVdsmImage(d) and drive.get('volumeID') == d.volumeID: + drive['truesize'] = str(d.truesize) + drive['apparentsize'] = str(d.apparentsize) + + data['_blockJobs'] = utils.picklecopy( + vm.conf.get('_blockJobs', {})) + + return data def all_vms(cif): diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index 39eee14..e36d497 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -270,9 +270,7 @@ self.cif = cif self.log = SimpleLogAdapter(self.log, {"vmId": self.conf['vmId']}) self._destroyed = False - self._recoveryLock = threading.Lock() - self._recoveryFile = constants.P_VDSM_RUN + \ - str(self.conf['vmId']) + '.recovery' + self._recoveryFile = recovery.VmState(str(self.conf['vmId'])) self._monitorResponse = 0 self.memCommitted = 0 self._consoleDisconnectAction = ConsoleDisconnectAction.LOCK_SCREEN @@ -334,6 +332,10 @@ self._numaInfo = {} self._vmJobs = None self._clientPort = '' + + @property + def start_time(self): + return self._startTime def _get_lastStatus(self): # note that we don't use _statusLock here. One of the reasons is the @@ -817,53 +819,7 @@ return base * (doubler + load) / doubler def saveState(self): - with self._recoveryLock(): - if self._recoveryFile is not None: - vmState = self._getVmState() - else: - vmState = None - if vmState is not None: - with tempfile.NamedTemporaryFile(dir=constants.P_VDSM_RUN, - delete=False) as f: - pickle.dump(vmState, f) - - os.rename(f.name, self._recoveryFile) - else: - self.log.debug('saveState after cleanup for status=%s', - self.lastStatus) - - try: - self._updateDomainDescriptor() - except Exception: - # we do not care if _dom suddenly died now - pass - - def _getVmState(self): - toSave = self.status() - toSave['startTime'] = self._startTime - if self.lastStatus != vmstatus.DOWN: - guestInfo = self.guestAgent.getGuestInfo() - toSave['username'] = guestInfo['username'] - toSave['guestIPs'] = guestInfo['guestIPs'] - toSave['guestFQDN'] = guestInfo['guestFQDN'] - else: - toSave['username'] = "" - toSave['guestIPs'] = "" - toSave['guestFQDN'] = "" - if 'sysprepInf' in toSave: - del toSave['sysprepInf'] - if 'floppy' in toSave: - del toSave['floppy'] - for drive in toSave.get('drives', []): - for d in self._devices[hwclass.DISK]: - if isVdsmImage(d) and drive.get('volumeID') == d.volumeID: - drive['truesize'] = str(d.truesize) - drive['apparentsize'] = str(d.apparentsize) - - toSave['_blockJobs'] = utils.picklecopy( - self.conf.get('_blockJobs', {})) - - return toSave + self._recoveryFile.save(self) def onReboot(self): try: @@ -1713,11 +1669,7 @@ con.cleanup() def _cleanupRecoveryFile(self): - with self._recoveryLock: - recovery_file = self._recoveryFile - self._recoveryFile = None - if recovery_file: - utils.rmFile(self._recoveryFile) + self._recoveryFile.cleanup() def _cleanupStatsCache(self): try: -- To view, visit https://gerrit.ovirt.org/51388 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6866eb2c3c9d5784c2054c860cbf0977e8e58031 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
