* Alon Bar-Lev alonbl@redhat.com [2012-09-05 16:11]:
Alon Bar-Lev has uploaded a new change for review.
Change subject: bootstrap: perform reboot asynchronously ......................................................................
bootstrap: perform reboot asynchronously
The use of /sbin/reboot may cause reboot to be performed at the middle of script execution.
Reboot should be delayed in background so that script will have a fair chance to terminate properly.
So, we fork and sleep 10 seconds? Is that really want we want to do? Why is 10 seconds enough?
Shouldn't the deployUtil be tracking the script execution and waiting for the scripts to complete before rebooting?
Change-Id: I0abb02ae4d5033a8b9f2d468da86fcdc53e2e1c2 Signed-off-by: Alon Bar-Lev alonbl@redhat.com
M vdsm_reg/deployUtil.py.in 1 file changed, 39 insertions(+), 5 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/83/7783/1
diff --git a/vdsm_reg/deployUtil.py.in b/vdsm_reg/deployUtil.py.in index ebc7d36..b72cb44 100644 --- a/vdsm_reg/deployUtil.py.in +++ b/vdsm_reg/deployUtil.py.in @@ -166,13 +166,47 @@
def reboot(): """
This function reboots the machine.
"""This function reboots the machine async
- fReturn = True
- fReturn = False
- out, err, ret = _logExec([EX_REBOOT])
- if ret:
fReturn = False
# Default maximum for the number of available file descriptors.
MAXFD = 1024
import resource # Resource usage information.
maxfd = resource.getrlimit(resource.RLIMIT_NOFILE)[1]
if (maxfd == resource.RLIM_INFINITY):
maxfd = MAXFD
try:
pid = os.fork()
if pid == 0:
try:
os.setsid()
for fd in range(0, maxfd):
try:
os.close(fd)
except OSError: # ERROR, fd wasn't open to begin with (ignored)
pass
os.open(os.devnull, os.O_RDWR) # standard input (0)
os.dup2(0, 1) # standard output (1)
os.dup2(0, 2) # standard error (2)
if os.fork() != 0:
os._exit(0)
time.sleep(10)
os.execl(EX_REBOOT, EX_REBOOT)
finally:
os._exit(1)
pid, status = os.waitpid(pid, 0)
if os.WIFEXITED(status) and os.WEXITSTATUS(status) == 0:
fReturn = True
except OSError:
pass
return fReturn
-- To view, visit http://gerrit.ovirt.org/7783 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: I0abb02ae4d5033a8b9f2d468da86fcdc53e2e1c2 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Alon Bar-Lev alonbl@redhat.com _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
----- Original Message -----
From: "Ryan Harper" ryanh@us.ibm.com To: "Alon Bar-Lev" alonbl@redhat.com Cc: vdsm-devel@lists.fedorahosted.org Sent: Friday, September 7, 2012 10:30:18 PM Subject: Re: Change in vdsm[master]: bootstrap: perform reboot asynchronously
- Alon Bar-Lev alonbl@redhat.com [2012-09-05 16:11]:
Alon Bar-Lev has uploaded a new change for review.
Change subject: bootstrap: perform reboot asynchronously ......................................................................
bootstrap: perform reboot asynchronously
The use of /sbin/reboot may cause reboot to be performed at the middle of script execution.
Reboot should be delayed in background so that script will have a fair chance to terminate properly.
So, we fork and sleep 10 seconds? Is that really want we want to do? Why is 10 seconds enough?
Shouldn't the deployUtil be tracking the script execution and waiting for the scripts to complete before rebooting?
Hi,
Reboot is called at the very end of the script, 10 seconds is more than enough.
You are right that we can track the pid of the bootstrap script's parent parent parent, but it will introduce more complexity that I am not sure worth it.
Regars, Alon
* Alon Bar-Lev alonbl@redhat.com [2012-09-07 14:45]:
----- Original Message -----
From: "Ryan Harper" ryanh@us.ibm.com To: "Alon Bar-Lev" alonbl@redhat.com Cc: vdsm-devel@lists.fedorahosted.org Sent: Friday, September 7, 2012 10:30:18 PM Subject: Re: Change in vdsm[master]: bootstrap: perform reboot asynchronously
- Alon Bar-Lev alonbl@redhat.com [2012-09-05 16:11]:
Alon Bar-Lev has uploaded a new change for review.
Change subject: bootstrap: perform reboot asynchronously ......................................................................
bootstrap: perform reboot asynchronously
The use of /sbin/reboot may cause reboot to be performed at the middle of script execution.
Reboot should be delayed in background so that script will have a fair chance to terminate properly.
So, we fork and sleep 10 seconds? Is that really want we want to do? Why is 10 seconds enough?
Shouldn't the deployUtil be tracking the script execution and waiting for the scripts to complete before rebooting?
Hi,
Reboot is called at the very end of the script, 10 seconds is more than enough.
I don't know how we can assert that... we're not the sole process on the box.
You are right that we can track the pid of the bootstrap script's parent parent parent, but it will introduce more complexity that I am not sure worth it.
Why can't we just wait on the PID if it we know it?
Regars, Alon
----- Original Message -----
From: "Ryan Harper" ryanh@us.ibm.com To: "Alon Bar-Lev" alonbl@redhat.com Cc: "Ryan Harper" ryanh@us.ibm.com, vdsm-devel@lists.fedorahosted.org Sent: Friday, September 7, 2012 10:47:10 PM Subject: Re: Change in vdsm[master]: bootstrap: perform reboot asynchronously
- Alon Bar-Lev alonbl@redhat.com [2012-09-07 14:45]:
----- Original Message -----
From: "Ryan Harper" ryanh@us.ibm.com To: "Alon Bar-Lev" alonbl@redhat.com Cc: vdsm-devel@lists.fedorahosted.org Sent: Friday, September 7, 2012 10:30:18 PM Subject: Re: Change in vdsm[master]: bootstrap: perform reboot asynchronously
- Alon Bar-Lev alonbl@redhat.com [2012-09-05 16:11]:
Alon Bar-Lev has uploaded a new change for review.
Change subject: bootstrap: perform reboot asynchronously ......................................................................
bootstrap: perform reboot asynchronously
The use of /sbin/reboot may cause reboot to be performed at the middle of script execution.
Reboot should be delayed in background so that script will have a fair chance to terminate properly.
So, we fork and sleep 10 seconds? Is that really want we want to do? Why is 10 seconds enough?
Shouldn't the deployUtil be tracking the script execution and waiting for the scripts to complete before rebooting?
Hi,
Reboot is called at the very end of the script, 10 seconds is more than enough.
I don't know how we can assert that... we're not the sole process on the box.
You are right that we can track the pid of the bootstrap script's parent parent parent, but it will introduce more complexity that I am not sure worth it.
Why can't we just wait on the PID if it we know it?
Because if we want to have this precise we need to track the following chain of processes.
sshd->sh->python->python
If we only track the last link in chain, it is not enough as we have race anyway, and have to wait some extra seconds, as the sh is doing some more logic and cleanups.
We can create the process tree which stop either at ssh or init... but even then if this is run differently we have a problem.
Alon.
On Fri, Sep 07, 2012 at 03:54:10PM -0400, Alon Bar-Lev wrote:
----- Original Message -----
From: "Ryan Harper" ryanh@us.ibm.com To: "Alon Bar-Lev" alonbl@redhat.com Cc: "Ryan Harper" ryanh@us.ibm.com, vdsm-devel@lists.fedorahosted.org Sent: Friday, September 7, 2012 10:47:10 PM Subject: Re: Change in vdsm[master]: bootstrap: perform reboot asynchronously
- Alon Bar-Lev alonbl@redhat.com [2012-09-07 14:45]:
----- Original Message -----
From: "Ryan Harper" ryanh@us.ibm.com To: "Alon Bar-Lev" alonbl@redhat.com Cc: vdsm-devel@lists.fedorahosted.org Sent: Friday, September 7, 2012 10:30:18 PM Subject: Re: Change in vdsm[master]: bootstrap: perform reboot asynchronously
- Alon Bar-Lev alonbl@redhat.com [2012-09-05 16:11]:
Alon Bar-Lev has uploaded a new change for review.
Change subject: bootstrap: perform reboot asynchronously ......................................................................
bootstrap: perform reboot asynchronously
The use of /sbin/reboot may cause reboot to be performed at the middle of script execution.
Reboot should be delayed in background so that script will have a fair chance to terminate properly.
So, we fork and sleep 10 seconds? Is that really want we want to do? Why is 10 seconds enough?
Shouldn't the deployUtil be tracking the script execution and waiting for the scripts to complete before rebooting?
Hi,
Reboot is called at the very end of the script, 10 seconds is more than enough.
I don't know how we can assert that... we're not the sole process on the box.
You are right that we can track the pid of the bootstrap script's parent parent parent, but it will introduce more complexity that I am not sure worth it.
Why can't we just wait on the PID if it we know it?
Because if we want to have this precise we need to track the following chain of processes.
sshd->sh->python->python
If we only track the last link in chain, it is not enough as we have race anyway, and have to wait some extra seconds, as the sh is doing some more logic and cleanups.
We can create the process tree which stop either at ssh or init... but even then if this is run differently we have a problem.
Ryan, I suppose you are right - hard-coding 10 seconds rings all fishiness bells. Still, the current behaviour, of running /sbin/reboot and hoping that it is slow enough so that the calling process writes what it needs to stdout, is even worse.
I trust Alon to think of a saner delayedReboot() in the near future, so I've taking the patch now, as a step in the right direction.
Regards, Dan.
* Dan Kenigsberg danken@redhat.com [2012-09-09 12:52]:
On Fri, Sep 07, 2012 at 03:54:10PM -0400, Alon Bar-Lev wrote:
----- Original Message -----
From: "Ryan Harper" ryanh@us.ibm.com To: "Alon Bar-Lev" alonbl@redhat.com Cc: "Ryan Harper" ryanh@us.ibm.com, vdsm-devel@lists.fedorahosted.org Sent: Friday, September 7, 2012 10:47:10 PM Subject: Re: Change in vdsm[master]: bootstrap: perform reboot asynchronously
- Alon Bar-Lev alonbl@redhat.com [2012-09-07 14:45]:
----- Original Message -----
From: "Ryan Harper" ryanh@us.ibm.com To: "Alon Bar-Lev" alonbl@redhat.com Cc: vdsm-devel@lists.fedorahosted.org Sent: Friday, September 7, 2012 10:30:18 PM Subject: Re: Change in vdsm[master]: bootstrap: perform reboot asynchronously
- Alon Bar-Lev alonbl@redhat.com [2012-09-05 16:11]:
Alon Bar-Lev has uploaded a new change for review.
Change subject: bootstrap: perform reboot asynchronously ......................................................................
bootstrap: perform reboot asynchronously
The use of /sbin/reboot may cause reboot to be performed at the middle of script execution.
Reboot should be delayed in background so that script will have a fair chance to terminate properly.
So, we fork and sleep 10 seconds? Is that really want we want to do? Why is 10 seconds enough?
Shouldn't the deployUtil be tracking the script execution and waiting for the scripts to complete before rebooting?
Hi,
Reboot is called at the very end of the script, 10 seconds is more than enough.
I don't know how we can assert that... we're not the sole process on the box.
You are right that we can track the pid of the bootstrap script's parent parent parent, but it will introduce more complexity that I am not sure worth it.
Why can't we just wait on the PID if it we know it?
Because if we want to have this precise we need to track the following chain of processes.
sshd->sh->python->python
If we only track the last link in chain, it is not enough as we have race anyway, and have to wait some extra seconds, as the sh is doing some more logic and cleanups.
We can create the process tree which stop either at ssh or init... but even then if this is run differently we have a problem.
Ryan, I suppose you are right - hard-coding 10 seconds rings all fishiness bells. Still, the current behaviour, of running /sbin/reboot and hoping that it is slow enough so that the calling process writes what it needs to stdout, is even worse.
I trust Alon to think of a saner delayedReboot() in the near future, so I've taking the patch now, as a step in the right direction.
Understood, something is better than nothing, however, it certainly is pushing the requirement for reworking delayedReboot() off a bit since it's "working" for now.
* Alon Bar-Lev alonbl@redhat.com [2012-09-07 15:46]:
----- Original Message -----
From: "Ryan Harper" ryanh@us.ibm.com To: "Alon Bar-Lev" alonbl@redhat.com Cc: "Ryan Harper" ryanh@us.ibm.com, vdsm-devel@lists.fedorahosted.org Sent: Friday, September 7, 2012 10:47:10 PM Subject: Re: Change in vdsm[master]: bootstrap: perform reboot asynchronously
- Alon Bar-Lev alonbl@redhat.com [2012-09-07 14:45]:
----- Original Message -----
From: "Ryan Harper" ryanh@us.ibm.com To: "Alon Bar-Lev" alonbl@redhat.com Cc: vdsm-devel@lists.fedorahosted.org Sent: Friday, September 7, 2012 10:30:18 PM Subject: Re: Change in vdsm[master]: bootstrap: perform reboot asynchronously
- Alon Bar-Lev alonbl@redhat.com [2012-09-05 16:11]:
Alon Bar-Lev has uploaded a new change for review.
Change subject: bootstrap: perform reboot asynchronously ......................................................................
bootstrap: perform reboot asynchronously
The use of /sbin/reboot may cause reboot to be performed at the middle of script execution.
Reboot should be delayed in background so that script will have a fair chance to terminate properly.
So, we fork and sleep 10 seconds? Is that really want we want to do? Why is 10 seconds enough?
Shouldn't the deployUtil be tracking the script execution and waiting for the scripts to complete before rebooting?
Hi,
Reboot is called at the very end of the script, 10 seconds is more than enough.
I don't know how we can assert that... we're not the sole process on the box.
You are right that we can track the pid of the bootstrap script's parent parent parent, but it will introduce more complexity that I am not sure worth it.
Why can't we just wait on the PID if it we know it?
Because if we want to have this precise we need to track the following chain of processes.
sshd->sh->python->python
If we only track the last link in chain, it is not enough as we have race anyway, and have to wait some extra seconds, as the sh is doing some more logic and cleanups.
We can create the process tree which stop either at ssh or init... but even then if this is run differently we have a problem.
what's wrong with subprocess/popen and not using sh ? Do we have something that requires sh? Can we rewrite that without sh ala vdsm-tool helpers?
vdsm-devel@lists.fedorahosted.org