Francesco Romani has posted comments on this change. Change subject: vm: improve safety between startup and shutdown ......................................................................
Patch Set 8: (2 comments) https://gerrit.ovirt.org/#/c/44989/8/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 251: vmdevices.graphics.initLegacyConf(self.conf) Line 252: self.cif = cif Line 253: self.log = SimpleLogAdapter(self.log, {"vmId": self.conf['vmId']}) Line 254: self._destroyed = threading.Event() Line 255: self._destroyRequested = threading.Event() > We shouldn't use camel case for attribute names, right? though call. On one hand, we should move (as fast as possible) towards pep8 compliancy. On the other hand, this module and this class are massively camelCased, so having mixed case is, IMO, the worst possible outcome. Line 256: self._recovery_file = recovery.File(self.conf['vmId']) Line 257: self._monitorResponse = 0 Line 258: self.memCommitted = 0 Line 259: self._consoleDisconnectAction = ConsoleDisconnectAction.LOCK_SCREEN Line 1306: try: Line 1307: self.guestAgent.stop() Line 1308: except Exception: Line 1309: pass Line 1310: self._created.set() > Setting this at two quite different places worries me a bit. E.g. didn't we I see your point. Let me think harder about a better solution. Line 1311: self.saveState() Line 1312: if event_data: Line 1313: self.send_status_event(**event_data) Line 1314: -- To view, visit https://gerrit.ovirt.org/44989 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8718f58f1d255d9e603db75aa1f256c03c300f3a Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Milan Zamazal <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
