Francesco Romani has posted comments on this change. Change subject: virt: migration: create full status in migration.py ......................................................................
Patch Set 1: (2 comments) https://gerrit.ovirt.org/#/c/43357/1/vdsm/API.py File vdsm/API.py: Line 353: try: Line 354: v = self._cif.vmContainer[self._UUID] Line 355: except KeyError: Line 356: return errCode['noVM'] Line 357: return v.migrateStatus() > This is used to get the status of a migration/hibernation in progress. The ok, but IIRC hooks run either before or after this API. Let me rephrase my question: how can this call possibly fail? What action can lead to failure here? I can't see how this is related to the hook you mentioned, because this method can run concurrently. The only way I can see for Vm.migrateStatus() to fail is if vm._dom.jobStats() fails. Line 358: Line 359: def getStats(self): Line 360: """ Line 361: Obtain statistics of the specified VM https://gerrit.ovirt.org/#/c/43357/1/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 100: 'status': { Line 101: 'code': 0, Line 102: 'message': 'Migration in progress'}, Line 103: 'migrationStats': { Line 104: 'progress': self._progress}} anyway, this patch can have its merit as cleanup, but in that case I'd rater construct migrationStats subdict on the fly in the getStat() method, instead of having it around all the time. Line 105: threading.Thread.__init__(self) Line 106: self._preparingMigrationEvt = True Line 107: self._migrationCanceledEvt = False Line 108: self._monitorThread = None -- To view, visit https://gerrit.ovirt.org/43357 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icbc1525f52dc5ee49647aa6d690f221f2dc96e5d Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Marcin Mirecki <mmire...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Marcin Mirecki <mmire...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
