Change in vdsm[master]: vdsm: Utilize CallbackChain in vm.shutdown

danken at redhat.com danken at redhat.com
Fri Mar 7 14:03:41 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: vdsm: Utilize CallbackChain in vm.shutdown
......................................................................


Patch Set 4: Code-Review-1

(3 comments)

http://gerrit.ovirt.org/#/c/23457/4//COMMIT_MSG
Commit Message:

Line 3: AuthorDate: 2014-01-17 16:14:20 +0100
Line 4: Commit:     Martin Betak <mbetak at redhat.com>
Line 5: CommitDate: 2014-03-05 16:59:55 +0100
Line 6: 
Line 7: vdsm: Utilize CallbackChain in vm.shutdown
this is an implementation detail. it is an important one, so it should be mentioned in the message, but it is not the subject of your patch. This patch is about trying to shutdown via ACPI if shutdown via the guest agent fails (and doing the same for reboot)
Line 8: 
Line 9: Refactor the existing shutdown/reboot code to utilize the
Line 10: utils.CallbackChain helper.
Line 11: 


http://gerrit.ovirt.org/#/c/23457/4/vdsm/vm.py
File vdsm/vm.py:

Line 1890: class StorageUnavailableError(Exception):
Line 1891:     pass
Line 1892: 
Line 1893: 
Line 1894: class VmPowerDown(object):
please move these new classes to a new module, say vmshutdown.py. This module is unbearably big.

Passing the sync event explicitly in the arg list would be nicer than touching a private member.

Please document the various arguments. In particular, document the difference between delay and timeout.
Line 1895:     def __init__(self, vm, delay, message, timeout):
Line 1896:         self.vm = vm
Line 1897:         self.chain = utils.CallbackChain()
Line 1898:         self.delay = delay


Line 1914:             # because we want to exit the CallbackChain in case either
Line 1915:             # of them happens
Line 1916:             self.vm._powerDownEvent.clear()
Line 1917: 
Line 1918:             self.vm._guestEventTime = time.time()
this changes an internal vm state - better leave it in vm.shutdown().
Line 1919:             self.vm._guestEvent = self.guestEvent
Line 1920: 
Line 1921:             self.chain.start()
Line 1922:             return {'status': {'code': doneCode['code'],


-- 
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: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Betak <mbetak at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap 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: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list