Change in vdsm[master]: Fix PEP8 violations in vm.py

smizrahi at redhat.com smizrahi at redhat.com
Thu Aug 23 23:52:04 UTC 2012


Saggi Mizrahi has uploaded a new change for review.

Change subject: Fix PEP8 violations in vm.py
......................................................................

Fix PEP8 violations in vm.py

Change-Id: I369a1dbfcf49cf1a8b1ac61fe88dd2fe3ea93b8e
Signed-off-by: Saggi Mizrahi <smizrahi at redhat.com>
---
M Makefile.am
M vdsm/vm.py
2 files changed, 118 insertions(+), 69 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/49/7449/1

diff --git a/Makefile.am b/Makefile.am
index 611fec4..7a07608c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -90,6 +90,7 @@
 	vdsm/storage/volume.py \
 	vdsm/supervdsm.py \
 	vdsm/supervdsmServer.py \
+	vdsm/vm.py \
 	vdsm_cli \
 	vdsm_hooks/fileinject/before_vm_start.py \
 	vdsm_hooks/promisc/after_vm_start.py \
diff --git a/vdsm/vm.py b/vdsm/vm.py
index 949a3ee..549eb8f 100644
--- a/vdsm/vm.py
+++ b/vdsm/vm.py
@@ -20,7 +20,9 @@
 
 import os
 import time
-import threading, logging
+import threading
+import logging
+import math
 from vdsm import constants
 import tempfile
 import pickle
@@ -50,6 +52,7 @@
 A module containing classes needed for VM communication.
 """
 
+
 def isVdsmImage(drive):
     return all(k in drive.keys() for k in ('volumeID', 'domainID', 'imageID',
                 'poolID'))
@@ -67,13 +70,17 @@
                  if not a.startswith('__')]
         return " ".join(attrs)
 
-class _MigrationError(RuntimeError): pass
+
+class _MigrationError(RuntimeError):
+    pass
+
 
 class MigrationSourceThread(threading.Thread):
     """
     A thread that takes care of migration on the source vdsm.
     """
     _ongoingMigrations = threading.BoundedSemaphore(1)
+
     @classmethod
     def setMaxOutgoingMigrations(cls, n):
         """Set the initial value of the _ongoingMigrations semaphore.
@@ -81,8 +88,8 @@
         must not be called after any vm has been run."""
         cls._ongoingMigrations = threading.BoundedSemaphore(n)
 
-    def __init__ (self, vm, dst='', dstparams='',
-                  mode='remote', method='online', **kwargs):
+    def __init__(self, vm, dst='', dstparams='', mode='remote',
+            method='online', **kwargs):
         self.log = vm.log
         self._vm = vm
         self._dst = dst
@@ -92,19 +99,23 @@
         self._machineParams = {}
         self._downtime = kwargs.get('downtime') or \
                             config.get('vars', 'migration_downtime')
-        self.status = {'status': {'code': 0, 'message': 'Migration in process'}, 'progress': 0}
+        self.status = {'status': {
+            'code': 0,
+            'message': 'Migration in process'}, 'progress': 0
+            }
         threading.Thread.__init__(self)
         self._preparingMigrationEvt = False
         self._migrationCanceledEvt = False
 
-    def getStat (self):
+    def getStat(self):
         """
         Get the status of the migration.
         """
         return self.status
 
     def _setupVdsConnection(self):
-        if self._mode == 'file': return
+        if self._mode == 'file':
+            return
         self.remoteHost = self._dst.split(':')[0]
         # FIXME: The port will depend on the binding being used.
         # This assumes xmlrpc
@@ -188,7 +199,11 @@
     def _finishSuccessfully(self):
         if self._mode != 'file':
             self._vm.setDownStatus(NORMAL, "Migration succeeded")
-            self.status = {'status': {'code': 0, 'message': 'Migration done'}, 'progress': 100}
+            self.status = {
+                    'status': {
+                        'code': 0,
+                        'message': 'Migration done'},
+                    'progress': 100}
         else:
             # don't pickle transient params
             for ignoreParam in ('displayIp', 'display', 'pid'):
@@ -203,7 +218,11 @@
                 self._vm.cif.teardownVolumePath(self._dstparams)
 
             self._vm.setDownStatus(NORMAL, "SaveState succeeded")
-            self.status = {'status': {'code': 0, 'message': 'SaveState done'}, 'progress': 100}
+            self.status = {
+                    'status': {
+                        'code': 0,
+                        'message': 'SaveState done'},
+                    'progress': 100}
 
     def run(self):
         try:
@@ -240,12 +259,15 @@
     def __str__(self):
         return "Bad volume specification " + RuntimeError.__str__(self)
 
-class DoubleDownError(RuntimeError): pass
+
+class DoubleDownError(RuntimeError):
+    pass
 
 VALID_STATES = ('Down', 'Migration Destination', 'Migration Source',
                 'Paused', 'Powering down', 'RebootInProgress',
                 'Restoring state', 'Saving State',
                 'Up', 'WaitForLaunch')
+
 
 class Vm(object):
     """
@@ -271,7 +293,7 @@
         self.conf = {'pid': '0'}
         self.conf.update(params)
         self.cif = cif
-        self.log = SimpleLogAdapter(self.log, {"vmId" : self.conf['vmId']})
+        self.log = SimpleLogAdapter(self.log, {"vmId": self.conf['vmId']})
         self.destroyed = False
         self._recoveryFile = constants.P_VDSM_RUN + str(
                                     self.conf['vmId']) + '.recovery'
@@ -304,7 +326,7 @@
         self._startTime = time.time() - float(
                                 self.conf.pop('elapsedTimeOffset', 0))
 
-        self._usedIndices = {} #{'ide': [], 'virtio' = []}
+        self._usedIndices = {}  # {'ide': [], 'virtio' = []}
         self._volumesPrepared = False
         self._pathsPreparedEvent = threading.Event()
         self._devices = {DISK_DEVICES: [], NIC_DEVICES: [],
@@ -312,9 +334,15 @@
                          CONTROLLER_DEVICES: [], GENERAL_DEVICES: [],
                          BALLOON_DEVICES: [], REDIR_DEVICES: []}
 
+        self.volumeUtilizationChunk = \
+                config.getint('irs', 'volume_utilization_chunk_mb')
+        self.systemShutdownTimeout = \
+                config.getint('vars', 'sys_shutdown_timeout')
+
     def _get_lastStatus(self):
         SHOW_PAUSED_STATES = ('Powering down', 'RebootInProgress', 'Up')
-        if not self._guestCpuRunning and self._lastStatus in SHOW_PAUSED_STATES:
+        if (not self._guestCpuRunning and
+                self._lastStatus in SHOW_PAUSED_STATES):
             return 'Paused'
         return self._lastStatus
 
@@ -344,7 +372,7 @@
     def _normalizeVdsmImg(self, drv):
         drv['needExtend'] = False
         drv['reqsize'] = drv.get('reqsize', '0')
-        if not drv.has_key('device'):
+        if not 'device' in drv:
             drv['device'] = 'disk'
 
         if drv['device'] == 'disk':
@@ -358,22 +386,26 @@
             drv['apparentsize'] = 0
 
     def __legacyDrives(self):
-         """
-         Backward compatibility for qa scripts that specify direct paths.
-         """
-         legacies = []
-         for index, linuxName in ((0, 'hda'), (1, 'hdb'), (2, 'hdc'), (3, 'hdd')):
-             path = self.conf.get(linuxName)
-             if path:
-                 legacies.append({'type': DISK_DEVICES, 'device': 'disk',
-                                  'path': path, 'iface': 'ide', 'index': index,
-                                  'truesize': 0})
-         return legacies
+        """
+        Backward compatibility for qa scripts that specify direct paths.
+        """
+        legacies = []
+        for index, linuxName in enumerate(('hda', 'hdb', 'hdc', 'hdd')):
+            path = self.conf.get(linuxName)
+            if path:
+                legacies.append({'type': DISK_DEVICES, 'device': 'disk',
+                                 'path': path, 'iface': 'ide', 'index': index,
+                                 'truesize': 0})
+        return legacies
 
     def __removableDrives(self):
-        removables =  [{'type': DISK_DEVICES, 'device': 'cdrom', 'iface': 'ide',
-                        'path': self.conf.get('cdrom', ''), 'index': 2,
-                        'truesize': 0}]
+        removables = [{
+            'type': DISK_DEVICES,
+            'device': 'cdrom',
+            'iface': 'ide',
+            'path': self.conf.get('cdrom', ''),
+            'index': 2,
+            'truesize': 0}]
         floppyPath = self.conf.get('floppy')
         if floppyPath:
             removables.append({'type': DISK_DEVICES, 'device': 'floppy',
@@ -440,14 +472,16 @@
         """
         controllers = []
         # For now we create by default only 'virtio-serial' controller
-        controllers.append({'type': CONTROLLER_DEVICES, 'device': 'virtio-serial'})
+        controllers.append({
+            'type': CONTROLLER_DEVICES,
+            'device': 'virtio-serial'})
         return controllers
 
     def getConfVideo(self):
         """
         Normalize video device provided by conf.
         """
-        vcards =[]
+        vcards = []
         if self.conf.get('display') == 'vnc':
             devType = 'cirrus'
         elif self.conf.get('display') == 'qxl':
@@ -480,9 +514,12 @@
         macs = self.conf.get('macAddr', '').split(',')
         models = self.conf.get('nicModel', '').split(',')
         bridges = self.conf.get('bridge', DEFAULT_BRIDGE).split(',')
-        if macs == ['']: macs = []
-        if models == ['']: models = []
-        if bridges == ['']: bridges = []
+        if macs == ['']:
+            macs = []
+        if models == ['']:
+            models = []
+        if bridges == ['']:
+            bridges = []
         if len(models) < len(macs) or len(models) < len(bridges):
             raise ValueError('Bad nic specification')
         if models and not (macs or bridges):
@@ -495,17 +532,20 @@
         for mac, model, bridge in zip(macs, models, bridges):
             if model == 'pv':
                 model = 'virtio'
-            nics.append({'type': NIC_DEVICES, 'macAddr': mac, 'nicModel': model,
-                         'network': bridge, 'device': 'bridge'})
+            nics.append({
+                'type': NIC_DEVICES,
+                'macAddr': mac,
+                'nicModel': model,
+                'network': bridge,
+                'device': 'bridge'})
         return nics
 
     def getConfDrives(self):
         """
         Normalize drives provided by conf.
         """
-        # FIXME
-        # Will be better to change the self.conf but this implies an API change.
-        # Remove this when the API parameters will be consistent.
+        # FIXME: Will be better to change the self.conf but this implies an API
+        # change. Remove this when the API parameters will be consistent.
         confDrives = self.conf['drives'] if self.conf.get('drives') else []
         if not confDrives:
             confDrives.extend(self.__legacyDrives())
@@ -536,7 +576,7 @@
         drives = [(order, drv) for order, drv in enumerate(confDrives)]
         indexed = []
         for order, drv in drives:
-            if not self._usedIndices.has_key(drv['iface']):
+            if drv['iface'] not in self._usedIndices:
                 self._usedIndices[drv['iface']] = []
             idx = drv.get('index')
             if idx is not None:
@@ -556,8 +596,8 @@
         """
         Reserve the required memory for this VM.
         """
-        self.memCommitted = 2**20 * (int(self.conf['memSize']) +
-                                config.getint('vars', 'guest_ram_overhead'))
+        self.memCommitted = ((2 ** 20) * (int(self.conf['memSize'])) +
+                config.getint('vars', 'guest_ram_overhead'))
 
     def _startUnderlyingVm(self):
         self.log.debug("Start")
@@ -653,7 +693,7 @@
             load = len(self.cif.vmContainer)
         return base * (doubler + load) / doubler
 
-    def saveState (self):
+    def saveState(self):
         if self.destroyed:
             return
         toSave = deepcopy(self.status())
@@ -666,7 +706,8 @@
             toSave['guestIPs'] = ""
         if 'sysprepInf' in toSave:
             del toSave['sysprepInf']
-            if 'floppy' in toSave: del toSave['floppy']
+            if 'floppy' in toSave:
+                del toSave['floppy']
         for drive in toSave.get('drives', []):
             for d in self._devices[DISK_DEVICES]:
                 if d.isVdsmImage() and drive.get('volumeID') == d.volumeID:
@@ -675,11 +716,11 @@
 
         with tempfile.NamedTemporaryFile(dir=constants.P_VDSM_RUN,
                                          delete=False) as f:
-             pickle.dump(toSave, f)
+            pickle.dump(toSave, f)
 
         os.rename(f.name, self._recoveryFile)
 
-    def onReboot (self, withRelaunch):
+    def onReboot(self, withRelaunch):
         try:
             self.log.debug('reboot event')
             self._startTime = time.time()
@@ -695,7 +736,7 @@
         except:
             self.log.error("Reboot event failed", exc_info=True)
 
-    def onShutdown (self):
+    def onShutdown(self):
         self.log.debug('onShutdown() event')
         self.user_destroy = True
 
@@ -719,12 +760,13 @@
     def _lvExtend(self, block_dev, newsize=None):
         volID = None
         for d in self._devices[DISK_DEVICES]:
-            if not d.blockDev: continue
-            if d.name != block_dev: continue
+            if not d.blockDev:
+                continue
+            if d.name != block_dev:
+                continue
             if newsize is None:
-                newsize = config.getint('irs',
-                    'volume_utilization_chunk_mb') + (d.apparentsize + 2**20
-                                                     - 1) / 2**20
+                apperantSizeMB = int(math.ceil(d.apparentsize / (2.0 ** 20.0)))
+                newsize = self.volumeUtilizationChunk + apperantSizeMB
             # TODO cap newsize by max volume size
             volDict = {'poolID': d.poolID, 'domainID': d.domainID,
                        'imageID': d.imageID, 'volumeID': d.volumeID,
@@ -732,11 +774,11 @@
             d.needExtend = True
             d.reqsize = newsize
             # sendExtendMsg expects size in bytes
-            self.cif.irs.sendExtendMsg(d.poolID, volDict, newsize * 2**20,
+            self.cif.irs.sendExtendMsg(d.poolID, volDict, newsize * (2 ** 20),
                                            self._afterLvExtend)
             self.log.debug('%s/%s (%s): apparentsize %s req %s', d.domainID,
-                           d.volumeID, d.name, d.apparentsize / constants.MEGAB,
-                           newsize) #in MiB
+                    d.volumeID, d.name, d.apparentsize / constants.MEGAB,
+                    newsize)  # in MiB
 
             volID = d.volumeID
             break
@@ -778,8 +820,8 @@
             apparentsize = int(res['apparentsize'])
             truesize = int(res['truesize'])
             self.log.debug('_afterLvExtend apparentsize %s req size %s',
-                            apparentsize / constants.MEGAB, d.reqsize) # MiB
-            if apparentsize >= d.reqsize * constants.MEGAB: #in Bytes
+                            apparentsize / constants.MEGAB, d.reqsize)  # MiB
+            if apparentsize >= d.reqsize * constants.MEGAB:  # in Bytes
                 d.needExtend = False
                 try:
                     self.cont()
@@ -821,8 +863,8 @@
             self._acquireCpuLockWithTimeout()
         try:
             if self.lastStatus in ('Migration Source', 'Saving State', 'Down'):
-                 self.log.error('cannot cont while %s', self.lastStatus)
-                 return errCode['unexpected']
+                self.log.error('cannot cont while %s', self.lastStatus)
+                return errCode['unexpected']
             self._underlyingCont()
             if hasattr(self, 'updateGuestCpuRunning'):
                 self.updateGuestCpuRunning()
@@ -860,7 +902,7 @@
                 self._guestEvent = 'Powering down'
                 self.log.debug('guestAgent shutdown called')
                 self.guestAgent.desktopShutdown(timeout, message)
-                agent_timeout = int(timeout) + config.getint('vars', 'sys_shutdown_timeout')
+                agent_timeout = int(timeout) + self.systemShutdownTimeout
                 timer = threading.Timer(agent_timeout, self._timedShutdown)
                 timer.start()
             elif utils.tobool(self.conf.get('acpiEnable', 'true')):
@@ -869,8 +911,10 @@
                 self._acpiShutdown()
             # No tools, no ACPI
             else:
-                return {'status': {'code': errCode['exist']['status']['code'],
-                        'message': 'VM without ACPI or active SolidICE tools. Try Forced Shutdown.'}}
+                return {'status': {
+                    'code': errCode['exist']['status']['code'],
+                    'message': 'VM without ACPI or active SolidICE tools. '
+                               'Try Forced Shutdown.'}}
         except:
             self.log.error("Shutdown failed", exc_info=True)
         return {'status': {'code': doneCode['code'],
@@ -912,7 +956,7 @@
         utils.rmFile(self._guestSocketFile)
         utils.rmFile(self._recoveryFile)
 
-    def setDownStatus (self, code, reason):
+    def setDownStatus(self, code, reason):
         try:
             self.lastStatus = 'Down'
             self.conf['exitCode'] = code
@@ -972,7 +1016,7 @@
                  'network': {}, 'disks': {},
                  'monitorResponse': str(self._monitorResponse),
                  'nice': self._nice,
-                 'elapsedTime' : str(int(time.time() - self._startTime)),
+                 'elapsedTime': str(int(time.time() - self._startTime)),
                  }
         if 'cdrom' in self.conf:
             stats['cdrom'] = self.conf['cdrom']
@@ -998,12 +1042,15 @@
                 try:
                     stats['disks'][var] = {}
                     for value in decStats[var]:
-                        stats['disks'][var][value] = utils.convertToStr(decStats[var][value])
+                        stats['disks'][var][value] = \
+                                utils.convertToStr(decStats[var][value])
                 except:
-                    self.log.error("Error setting vm disk stats", exc_info=True)
+                    self.log.error("Error setting vm disk stats",
+                            exc_info=True)
 
-
-        if self.lastStatus in ('Saving State', 'Restoring state', 'Migration Source', 'Migration Destination', 'Paused'):
+        if self.lastStatus in ('Saving State',
+                'Restoring state', 'Migration Source',
+                'Migration Destination', 'Paused'):
             stats['status'] = self.lastStatus
         elif self.isMigrating():
             if self._migrationSourceThread._mode == 'file':
@@ -1026,8 +1073,9 @@
         memUsage = 0
         realMemUsage = int(stats['memUsage'])
         if realMemUsage != 0:
-            memUsage = 100 - float(realMemUsage) / int(self.conf['memSize']) * 100
-        stats['memUsage'] = utils.convertToStr(int(memUsage))
+            memUsage = 1 - (float(realMemUsage) / int(self.conf['memSize']))
+
+        stats['memUsage'] = utils.convertToStr(int(memUsage * 100))
         return stats
 
     def isMigrating(self):


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I369a1dbfcf49cf1a8b1ac61fe88dd2fe3ea93b8e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi at redhat.com>


More information about the vdsm-patches mailing list