Change in vdsm[master]: virt: Do not perform disconnect actions when destroyed

fromani at redhat.com fromani at redhat.com
Wed Jun 17 07:03:01 UTC 2015


Francesco Romani has posted comments on this change.

Change subject: virt: Do not perform disconnect actions when destroyed
......................................................................


Patch Set 3: Code-Review-1

(1 comment)

looks OK (-1 for visibility only). Please reiterate using a comment near to the code that we know this is racy and this is not the definitive fix.

Yes, this will partially repeat the content of the commit message, but we dealing with this kind of bugs I prefer a little redundancy to make sure our future selves are aware of what we thought back in time.

https://gerrit.ovirt.org/#/c/41373/3/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 856:         if clientIp:
Line 857:             self.conf['clientIp'] = clientIp
Line 858: 
Line 859:     def _timedDesktopLock(self):
Line 860:         if not self.conf.get('clientIp', '') and not self._destroyed:
Please add one-line(-ish) comment stating that we know this is still racy and this is only a quick fix.
Line 861:             delay = config.get('vars', 'user_shutdown_timeout')
Line 862:             timeout = config.getint('vars', 'sys_shutdown_timeout')
Line 863:             CDA = ConsoleDisconnectAction
Line 864:             if self._consoleDisconnectAction == CDA.LOCK_SCREEN:


-- 
To view, visit https://gerrit.ovirt.org/41373
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I34d384a057e2d6b9093b5ad9ce67d0ba33c42264
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek <mskrivan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list