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