Francesco Romani has posted comments on this change. Change subject: vdsm: Add force and timeout options to VM.shutdown ......................................................................
Patch Set 4: Code-Review-1 (7 comments) A few small remarks, -1 just for visibility 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. 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 (and if I'm correct the above line(s) should be fixed as well - of course in a different patch) 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() ? 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 nicer IMO). 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 just think of destroyVm which is only slightly better. 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 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 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 <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Martin Betak <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
