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 <from...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Michal Skrivanek <mskri...@redhat.com> Gerrit-Reviewer: Milan Zamazal <mzama...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
