Change in vdsm[master]: vdsm: Utilize CallbackChain in vm.shutdown
mbetak at redhat.com
mbetak at redhat.com
Thu Jan 30 15:16:01 UTC 2014
Martin Betak has posted comments on this change.
Change subject: vdsm: Utilize CallbackChain in vm.shutdown
......................................................................
Patch Set 2:
(5 comments)
http://gerrit.ovirt.org/#/c/23457/2/vdsm/vm.py
File vdsm/vm.py:
Line 2617
Line 2618
Line 2619
Line 2620
Line 2621
> Is this got lost in the refactoring or is no longer needed?
Yeah, I guess let's not spawn unnecessary threads, even though the callbackchain should be safe when the VM is down.
Line 2659
Line 2660
Line 2661
Line 2662
Line 2663
> I guess this method can no be deleted, correct?
Done
Line 1864: if vm.guestAgent and vm.guestAgent.isResponsive():
Line 1865: self.chain.addCallback(callbacks['guestAgent'], delay, message)
Line 1866:
Line 1867: # then acpi if enabled
Line 1868: if utils.tobool(vm.conf.get('acpiEnable', 'true')):
> Since we're cleaning up things, why not just True instead of 'true'?
utils.tobool is case invariant and I didn't see particular reason to break consistency with the pattern utils.tobool(conf.get(..., 'true')) used all around vm.py
Line 1869: self.chain.addCallback(callbacks['acpi'])
Line 1870:
Line 1871: def start(self):
Line 1872: # are there any available methods for shutdown/reboot?
Line 2018:
Line 2019: if (self.arch not in ['ppc64', 'x86_64']):
Line 2020: raise RuntimeError('Unsupported architecture: %s' % self.arch)
Line 2021:
Line 2022: self._powerDownEvent = threading.Event()
> not sure if best to have this here or _onQemuDeath
Where would you suggest for the _powerDownEvent object to be constructed? In _onQemuDeath we need to signalize it and it needs to be constructed even before the first call of wait()
Line 2023:
Line 2024: def _get_lastStatus(self):
Line 2025: PAUSED_STATES = ('Powering down', 'RebootInProgress', 'Up')
Line 2026: if not self._guestCpuRunning and self._lastStatus in PAUSED_STATES:
Line 2458: self.log.debug('ejected volatileFloppy')
Line 2459: except Exception:
Line 2460: self.log.error("Reboot event failed", exc_info=True)
Line 2461:
Line 2462: def onShutdown(self):
> this seems no longer called (even before your patch!) according to git grep
Done
Line 2463: self.log.debug('onShutdown() event')
Line 2464: self.user_destroy = True
Line 2465:
Line 2466: def onConnect(self, clientIp=''):
--
To view, visit http://gerrit.ovirt.org/23457
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8cc4f4140913df9827cca296e880b37f47a21b76
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Betak <mbetak at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Martin Betak <mbetak at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list