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