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

zhshzhou at linux.vnet.ibm.com zhshzhou at linux.vnet.ibm.com
Fri Aug 24 08:10:22 UTC 2012


Zhou Zheng Sheng has posted comments on this change.

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


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

(10 inline comments)

....................................................
File vdsm/vm.py
Line 101:                             config.get('vars', 'migration_downtime')
Line 102:         self.status = {'status': {
Line 103:             'code': 0,
Line 104:             'message': 'Migration in process'}, 'progress': 0
Line 105:             }
I think it can be
self.status = {'status': {'code': 0,
                          'message': 'Migration in process'},
               'progress': 0}

or

self.status = {'status':
                {'code': 0,
                 'message': 'Migration in process'},
               'progress': 0}
Line 106:         threading.Thread.__init__(self)
Line 107:         self._preparingMigrationEvt = False
Line 108:         self._migrationCanceledEvt = False
Line 109: 


Line 125:         except:
Line 126:             pass
Line 127:         serverAddress = self.remoteHost + ':' + self.remotePort
Line 128:         if config.getboolean('vars', 'ssl'):
Line 129:             self.destServer = vdscli.connect(serverAddress, useSSL=True,
How about

self.destServer = vdscli.connect(
        serverAddress, useSSL=True,
        TransportClass=kaxmlrpclib.TcpkeepSafeTransport)
Line 130:                     TransportClass=kaxmlrpclib.TcpkeepSafeTransport)
Line 131:         else:
Line 132:             self.destServer = kaxmlrpclib.Server('http://' + serverAddress)
Line 133:         self.log.debug('Destination server is: ' + serverAddress)


Line 172:                     break
Line 173:                 time.sleep(1)
Line 174:                 lockTimeout -= 1
Line 175:                 if lockTimeout == 0:
Line 176:                     self.log.warning('Agent ' + self._vm.id +
Same as line 130.
Line 177:                             ' unresponsive. Hiberanting without desktopLock.')
Line 178:                     break
Line 179:             self._vm.pause('Saving State')
Line 180:         else:


Line 233:             self.status['progress'] = 10
Line 234:             MigrationSourceThread._ongoingMigrations.acquire()
Line 235:             try:
Line 236:                 self.log.debug("migration semaphore acquired")
Line 237:                 if not mstate:
Same as line 130.
Line 238:                     self._vm.conf['_migrationParams'] = {'dst': self._dst,
Line 239:                                 'mode': self._mode, 'method': self._method,
Line 240:                                 'dstparams': self._dstparams}
Line 241:                     self._vm.saveState()


Line 241:                     self._vm.saveState()
Line 242:                     self._startUnderlyingMigration()
Line 243:                 self._finishSuccessfully()
Line 244:             except libvirt.libvirtError, e:
Line 245:                 if e.get_error_code() == libvirt.VIR_ERR_OPERATION_ABORTED:
Same as line 130.... I will skip similar cases.
Line 246:                     self.status = {'status': {'code': errCode['migCancelErr'],
Line 247:                         'message': 'Migration canceled'}}
Line 248:                 raise
Line 249:             finally:


Line 341: 
Line 342:     def _get_lastStatus(self):
Line 343:         SHOW_PAUSED_STATES = ('Powering down', 'RebootInProgress', 'Up')
Line 344:         if (not self._guestCpuRunning and
Line 345:                 self._lastStatus in SHOW_PAUSED_STATES):
Can "self" in the second line align to "not" in the upper line?
Line 346:             return 'Paused'
Line 347:         return self._lastStatus
Line 348: 
Line 349:     def _set_lastStatus(self, value):


Line 406
Line 407
Line 408
Line 409
Line 410
Is it better if we write "if self.conf.get('devices') is None" ?


Line 606:             self._ongoingCreations.acquire()
Line 607:             self.log.debug("_ongoingCreations acquired")
Line 608:             try:
Line 609:                 self._run()
Line 610:                 if self.lastStatus != 'Down' and 'recover' not in self.conf \
I think it could be

if (self.lastStatus != 'Down' and
    'recover' not in self.conf and
    not self.cif.mom):
Line 611:                         and not self.cif.mom:
Line 612:                     # If MOM is available, we needn't tell it to adjust KSM
Line 613:                     # behaviors on VM start/destroy, because the tuning can be
Line 614:                     # done automatically acccording its statistical data.


Line 621:             finally:
Line 622:                 self._ongoingCreations.release()
Line 623:                 self.log.debug("_ongoingCreations released")
Line 624: 
Line 625:             if ('migrationDest' in self.conf or 'restoreState' in self.conf
The upper 2 lines are not very nice...
Line 626:                                                ) and self.lastStatus != 'Down':
Line 627:                 self._waitForIncomingMigrationFinish()
Line 628: 
Line 629:             self.lastStatus = 'Up'


Line 1090:             # while we were blocking, another migrationSourceThread could have
Line 1091:             # taken self Down
Line 1092:             if self._lastStatus == 'Down':
Line 1093:                 return errCode['noVM']
Line 1094:             self._migrationSourceThread = self.MigrationSourceThreadClass(self,
I think it's OK to use back slash here

self._migrationSourceThread = \
        self.MigrationSourceThreadClass(self, **params)
Line 1095:                                                                      **params)
Line 1096:             self._migrationSourceThread.start()
Line 1097:             check = self._migrationSourceThread.getStat()
Line 1098:             if check['status']['code']:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I369a1dbfcf49cf1a8b1ac61fe88dd2fe3ea93b8e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list