Michal Skrivanek has posted comments on this change.
Change subject: migrateStatus() progress report ......................................................................
Patch Set 11: (8 inline comments)
couple more comments...
.................................................... File vdsm/vm.py Line 105: config.get('vars', 'migration_downtime') Line 106: self.status = { Line 107: 'status': { Line 108: 'code': 0, Line 109: 'message': 'Migration in process'}, shouldn't it be "in progress"? Or see comment at migrate() function later Line 110: 'progress': 0} Line 111: threading.Thread.__init__(self) Line 112: self._preparingMigrationEvt = False Line 113: self._migrationCanceledEvt = False
Line 116: def getStat(self): Line 117: """ Line 118: Get the status of the migration. Line 119: """ Line 120: if self._monitorThread is not None: would agree with peet, we would like to know at what % the migration failed. Line 121: # fetch migration status from the monitor thread Line 122: self.status['progress'] = int( Line 123: float(self._monitorThread.data_progress + Line 124: self._monitorThread.mem_progress) / 2)
Line 192: ' unresponsive. Hiberanting without ' Line 193: 'desktopLock.') Line 194: break Line 195: self._vm.pause('Saving State') Line 196: else: just a log, but still - "process" Line 197: self.log.debug("migration Process begins") Line 198: self._vm.lastStatus = 'Migration Source' Line 199: Line 200: def _recover(self, message):
Line 234: self._vm.cif.teardownVolumePath(self._dstparams) Line 235: Line 236: self._vm.setDownStatus(NORMAL, "SaveState succeeded") Line 237: self.status = {'status': { Line 238: 'code': 0, pls fix indentation Line 239: 'message': 'SaveState done'}, Line 240: 'progress': 100} Line 241: Line 242: def _patchConfigForLegacy(self):
Line 255 Line 256 Line 257 Line 258 Line 259 I guess this is ok and needed to prevent jumping back since we are taking stats from libvirt. But still - if the preparation steps can take significant time(do they?) we may actually want to keep 10 hardcoded and progress would be recalculated to 10+0.9*progress
Line 277: except libvirt.libvirtError, e: Line 278: if e.get_error_code() == libvirt.VIR_ERR_OPERATION_ABORTED: Line 279: self.status = { Line 280: 'status': { Line 281: 'code': errCode['migCancelErr'], is it ok progress is missing here? why? Line 282: 'message': 'Migration canceled'}} Line 283: raise Line 284: finally: Line 285: if '_migrationParams' in self._vm.conf:
Line 1183: self._migrationSourceThread = \ Line 1184: self.MigrationSourceThreadClass(self, **params) Line 1185: self._migrationSourceThread.start() Line 1186: check = self._migrationSourceThread.getStat() Line 1187: if check['status']['code']: since we are apparently ignoring the returncode anyway, how about getStat() before line 1185 and set the right initial message at line 109 Line 1188: return check Line 1189: return {'status': {'code': 0, Line 1190: 'message': 'Migration process starting'}} Line 1191: finally:
Line 1197: def migrateCancel(self): Line 1198: self._acquireCpuLockWithTimeout() Line 1199: try: Line 1200: self._migrationSourceThread.stop() Line 1201: return {'status': {'code': 0, also progress? Line 1202: 'message': 'Migration process stopped'}} Line 1203: except libvirt.libvirtError, e: Line 1204: if e.get_error_code() == libvirt.VIR_ERR_OPERATION_INVALID: Line 1205: return errCode['migCancelErr']
-- To view, visit http://gerrit.ovirt.org/6824 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I3ff00e85c88e865cd81697d427d6bd5473e0f79e Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Peter V. Saveliev peet@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: Peter V. Saveliev peet@redhat.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra evilissimo@gmail.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server