Change in vdsm[master]: virt: move handling of recovery file in a class

fromani at redhat.com fromani at redhat.com
Tue Jan 5 14:18:54 UTC 2016


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 <fromani at redhat.com>
---
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 <fromani at redhat.com>


More information about the vdsm-patches mailing list