[node-patches] Change in ovirt-node[master]: Update reboot task to run async from installer and install m...

[fabiand at fedoraproject.org]

Fabian Deutsch has posted comments on this change.

Change subject: Update reboot task to run async from installer and install module
......................................................................


Patch Set 5: I would prefer that you didn't submit this

(5 inline comments)

Two things:
1)
system.reboot() shall just be a very simple process.call("reboot") (to make this function really reliable)
this function should be called in most places.

2)
system.async_reboot(delay) this is a function using the daemon to initiate the reboot.
the delay needs to happen in the daemon context to give the caller time to exit cleanly. before the reboot kicks in.

....................................................
File src/ovirt/node/installer/core/progress_page.py
Line 69:         pass
Line 70: 
Line 71:     def on_merge(self, effective_changes):
Line 72:         if "action.reboot" in effective_changes:
Line 73:             utils.system.async_reboot()
why don't you call system.reboot() here?
Line 74: 
Line 75: 
Line 76: class InstallerThread(threading.Thread):
Line 77:     ui_thread = None


....................................................
File src/ovirt/node/setup/core/status_page.py
Line 142:             self.application.quit()
Line 143: 
Line 144:         elif "action.restart" in changes:
Line 145:             self.logger.info("Restarting")
Line 146:             self.dry_or(lambda: system.async_reboot())
Why don't you call system.reboot() here?
Line 147: 
Line 148:         elif "action.poweroff" in changes:
Line 149:             self.logger.info("Shutting down")
Line 150:             self.dry_or(lambda: system.poweroff())


....................................................
File src/ovirt/node/utils/system.py
Line 34: 
Line 35: 
Line 36: def reboot():
Line 37:     """Reboot the system
Line 38:     """
actually this shall just call process.call("reboot")
Line 39:     reboot_task = Reboot()
Line 40:     reboot_task.reboot()
Line 41: 
Line 42: 


Line 39:     reboot_task = Reboot()
Line 40:     reboot_task.reboot()
Line 41: 
Line 42: 
Line 43: def async_reboot(delay=3):
reboot_task = Reboot()
    reboot_task.reboot()
should go in here.

The delay should happen inside the daemon.
This is crucial. to give the caller of the function e.g. the update script or whovere want's to reboot, the time to exit, and let the reboot then kick off.
Line 44:     time.sleep(delay)
Line 45:     reboot()
Line 46: 
Line 47: 


Line 314: 
Line 315:     def rebootMain(self, reboot):
Line 316:         os.execl(reboot, reboot)
Line 317: 
Line 318:     def reboot(self):
the delay needs to be added here. iiuic
Line 319:         try:
Line 320:             import daemon
Line 321:             with daemon.DaemonContext():
Line 322:                 # the following lines are all executed in a background daemon


--
To view, visit http://gerrit.ovirt.org/14491
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1b4e048ff56b8c9ebd49a82b3d5546c7c326df8
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Joey Boggs <jboggs at redhat.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at fedoraproject.org>
Gerrit-Reviewer: Joey Boggs <jboggs at redhat.com>