Francesco Romani has posted comments on this change. Change subject: Getting migration statistics NPE ......................................................................
Patch Set 1: (2 comments) https://gerrit.ovirt.org/#/c/40657/1//COMMIT_MSG Commit Message: Line 6: Line 7: Getting migration statistics NPE Line 8: Line 9: MigrateStats only return progress when monitor thread is alive, Line 10: When calling MigrateStats after the thread is dead we get NPE. s/NPE/KeyError/ still bad in this case, indeed, but a bit less dramatic :) Line 11: (when calling getMigrationStatus verb) Line 12: Line 13: Change-Id: I9277b70432d1d9ecc3c39ee7deabbbd89d179715 https://gerrit.ovirt.org/#/c/40657/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 1298: 'disks': {}} Line 1299: if 'pauseCode' in self.conf: Line 1300: stats['pauseCode'] = self.conf['pauseCode'] Line 1301: if self.isMigrating() and 'progress' in self.migrateStatus(): Line 1302: stats['migrationProgress'] = self.migrateStatus()['progress'] migrateStatus() could be made safer (https://gerrit.ovirt.org/#/q/status:open+project:vdsm+branch:master+topic:cleanup_migration), so this is of little help, but still I prefer to do like: if self.isMigrating(): mig_status = self.migrateStatus() if 'progress' in mig_status: stats['migrationProgress'] = mig_status['progress'] Not sure if the so-called "Better to Ask Forgiveness than Permission" idiom is actually better _here_: if self.isMigrating(): mig_status = self.migrateStatus() try: stats['migrationProgress'] = mig_status['progress'] except KeyError: pass # not critical So my request is just to call 'migrateStatus()' once, everything else -naming included- at your best judgement. Line 1303: Line 1304: decStats = {} Line 1305: try: Line 1306: vm_sample = sampling.stats_cache.get(self.id) -- To view, visit https://gerrit.ovirt.org/40657 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9277b70432d1d9ecc3c39ee7deabbbd89d179715 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Shahar Havivi <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
