Change in vdsm[master]: vdsm: Add force and timeout options to VM.shutdown
mbetak at redhat.com
mbetak at redhat.com
Mon May 12 12:34:06 UTC 2014
Martin Betak has posted comments on this change.
Change subject: vdsm: Add force and timeout options to VM.shutdown
......................................................................
Patch Set 4:
(7 comments)
http://gerrit.ovirt.org/#/c/27054/4/lib/vdsm/config.py.in
File lib/vdsm/config.py.in:
Line 124:
Line 125: ('max_outgoing_migrations', '3',
Line 126: 'Maximum concurrent outgoing migrations'),
Line 127:
Line 128: ('sys_shutdown_timeout', '120',
> Why is this needed? I don't get it.
The old default was too low. 10 seconds it too short a grace period for even medium VMs.
Line 129: 'Destroy and shutdown timeouts (in sec) before completing the '
Line 130: 'action.'),
Line 131:
Line 132: ('user_shutdown_timeout', '30',
http://gerrit.ovirt.org/#/c/27054/4/vdsm/API.py
File vdsm/API.py:
Line 642: if not delay:
Line 643: delay = config.get('vars', 'user_shutdown_timeout')
Line 644: if not message:
Line 645: message = USER_SHUTDOWN_MESSAGE
Line 646: if not timeout:
> IMO better: timeout is None
Not sure about this. A quick git grep for both variants reveals a strong prevalence of the `if not` form. Maybe Danken could clarify what is the preferred form.
Line 647: timeout = config.get('vars', 'sys_shutdown_timeout')
Line 648: return v.shutdown(delay, message, reboot, timeout, force)
Line 649:
Line 650: def _createSysprepFloppyFromInf(self, infFileBinary, floppyImage):
Line 643: delay = config.get('vars', 'user_shutdown_timeout')
Line 644: if not message:
Line 645: message = USER_SHUTDOWN_MESSAGE
Line 646: if not timeout:
Line 647: timeout = config.get('vars', 'sys_shutdown_timeout')
> getint() ?
Done
Line 648: return v.shutdown(delay, message, reboot, timeout, force)
Line 649:
Line 650: def _createSysprepFloppyFromInf(self, infFileBinary, floppyImage):
Line 651: try:
http://gerrit.ovirt.org/#/c/27054/4/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 2301: if self.lastStatus == vmstatus.DOWN:
Line 2302: return errCode['noVM']
Line 2303:
Line 2304: delay = int(delay)
Line 2305: timeout = int(timeout)
> this will become rendundant if we use getint (and it becomes a little bit n
Done
Line 2306:
Line 2307: self._guestEventTime = time.time()
Line 2308: if reboot:
Line 2309: self._guestEvent = vmstatus.REBOOT_IN_PROGRESS
Line 4326: self.deleteVm()
Line 4327:
Line 4328: return {'status': doneCode}
Line 4329:
Line 4330: def doDestroy(self):
> I'd like a better name... Unfortunately I'm short of suggestions. I can jus
from the top of my head I can think of perhaps: 'performDestroy' or 'destroyDomain'?
Line 4331: for dev in self._customDevices():
Line 4332: hooks.before_device_destroy(dev._deviceXML, self.conf,
Line 4333: dev.custom)
Line 4334:
http://gerrit.ovirt.org/#/c/27054/4/vdsm_api/vdsmapi-schema.json
File vdsm_api/vdsmapi-schema.json:
Line 6420: #
Line 6421: # @reboot: #optional True if reboot is desired, False for shutdown
Line 6422: #
Line 6423: # @timeout: #optional Number of seconds to wait before trying next
Line 6424: # shutdown/reboot method.
> Please add a (new in version whatever) line here
Done
Line 6425: #
Line 6426: # @force: #optional True if shutdown/reboot desired by any means necessary
Line 6427: # (forceful reboot/shutdown if all graceful methods fail)
Line 6428: # Default is False (i.e. graceful only).
Line 6423: # @timeout: #optional Number of seconds to wait before trying next
Line 6424: # shutdown/reboot method.
Line 6425: #
Line 6426: # @force: #optional True if shutdown/reboot desired by any means necessary
Line 6427: # (forceful reboot/shutdown if all graceful methods fail)
> ditto
Done
Line 6428: # Default is False (i.e. graceful only).
Line 6429: # Since: 4.10.0
Line 6430: ##
Line 6431: {'command': {'class': 'VM', 'name': 'shutdown'},
--
To view, visit http://gerrit.ovirt.org/27054
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I30180f9906eb45fa485aa7d773a436e62bd3c815
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