setpgid() before exec'ing qemu
Hi I am writing a python test suite that uses oz (https://github.com/clalancette/oz) which uses libquestfs. I am simulating a small cloud and have some apps that need to talk to the guests. All works really well until I needed to start another process. So I start a process then "oz" starts up a VM (it uses libquestfs) then when "oz" cleans up it calls libquestfs.kill_subprocess(). At this point my test locks up. A bacetrace of the python code is: #0 0x00000036fe80ef1e in waitpid () from /lib64/libpthread.so.0 #1 0x0000003629c0df7f in guestfs_close () from /usr/lib64/libguestfs.so.0 then a mess of python stuff If I attach to it with strace it shows: wait4(0, It looks like libguestfs's children are already dead pstree 13107 python─┬─cped───3*[{cped}] └─qpidd───9*[{qpidd}] so it's seems like it's waiting on my process to die. So I changed the oz code to call close (in python "del obj") and all is well, however I was looking in libquestfs code and saw: #if 0 /* Set up a new process group, so we can signal this process * and all subprocesses (eg. if qemu is really a shell script). */ setpgid (0, 0); #endif Any reason this is "if def'ed" out? this was done in commit 88f69eb03160a62d38 I was thinking this might prevent my problem from occuring in the first place? Chris: Any special reason you are using kill_subprocess()? def kill_subprocess (self): u"""This kills the qemu subprocess. You should never need to call this. """ return libguestfsmod.kill_subprocess (self._o) Regards Angus Salkeld
On Fri, Mar 11, 2011 at 12:56:43PM +1100, Angus Salkeld wrote:
Hi
I am writing a python test suite that uses oz (https://github.com/clalancette/oz) which uses libquestfs. I am simulating a small cloud and have some apps that need to talk to the guests.
All works really well until I needed to start another process. So I start a process then "oz" starts up a VM (it uses libquestfs) then when "oz" cleans up it calls libquestfs.kill_subprocess(). At this point my test locks up.
As Chris says, any reason to be using kill_subprocess at all? You can just delete the handle which has the same effect (and is a much more well-tested path). Something like: del g
A bacetrace of the python code is: #0 0x00000036fe80ef1e in waitpid () from /lib64/libpthread.so.0 #1 0x0000003629c0df7f in guestfs_close () from /usr/lib64/libguestfs.so.0 then a mess of python stuff
If I attach to it with strace it shows: wait4(0,
It looks like libguestfs's children are already dead pstree 13107 python─┬─cped───3*[{cped}] └─qpidd───9*[{qpidd}]
so it's seems like it's waiting on my process to die.
So I changed the oz code to call close (in python "del obj") and all is well, however I was looking in libquestfs code and saw: #if 0 /* Set up a new process group, so we can signal this process * and all subprocesses (eg. if qemu is really a shell script). */ setpgid (0, 0); #endif
Any reason this is "if def'ed" out? this was done in commit 88f69eb03160a62d38 I was thinking this might prevent my problem from occuring in the first place?
I don't know why exactly waitpid isn't working, but I doubt it's anything to do with the (lack of) a process group. Are you using a qemu wrapper script? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
On Fri, Mar 11, 2011 at 05:26:50PM +0000, Richard W.M. Jones wrote:
As Chris says, any reason to be using kill_subprocess at all?
Actually as Chris *didn't* say, I was reading that wrong :-) Anyway, I'd try deleting the handle. AFAIK Python will run the destructor immediately, and the path taken via guestfs_close certainly kills the subprocess: http://git.annexia.org/?p=libguestfs.git;a=blob;f=src/guestfs.c;h=8b7ab4d771... If you're really worried about the subprocess staying around, then grab the PID (g.get_pid) and kill the process *after* deleting the handle. (This is way outside the scope of the API ...) Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top
On Fri, Mar 11, 2011 at 05:26:50PM +0000, Richard W.M. Jones wrote:
As Chris says, any reason to be using kill_subprocess at all?
Actually as Chris *didn't* say, I was reading that wrong :-)
Anyway, I'd try deleting the handle. AFAIK Python will run the destructor immediately, and the path taken via guestfs_close certainly kills the subprocess:
http://git.annexia.org/?p=libguestfs.git;a=blob;f=src/guestfs.c;h=8b7ab4d771...
If you're really worried about the subprocess staying around, then grab the PID (g.get_pid) and kill the process *after* deleting the handle. (This is way outside the scope of the API ...) We could do this but I think there is a little bug in libguestfs
On Fri, Mar 11, 2011 at 05:43:52PM +0000, Richard W.M. Jones wrote: that we can easily fix. Attached is a simple python test case that fails for me. I believe that the kill_subprocess() is killing what it should then guestfs_close() is getting run again because "del g" calls obj.__del__() and that calls guestfs_close(). It this code there is a waitpid() without a check to see if the pid is > 0. (I'll reply to this email with a patch) Can someone (with a libguestfs that builds) test this? I have been having problems setting up a libguestfs that can build else I'de test this my self. Thanks Angus Salkeld
Rich.
-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top
Signed-off-by: Angus Salkeld <asalkeld@redhat.com> --- src/launch.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/launch.c b/src/launch.c index e50985d..8de2857 100644 --- a/src/launch.c +++ b/src/launch.c @@ -769,7 +769,7 @@ launch_appliance (guestfs_h *g) } if (g->pid > 0) kill (g->pid, 9); if (g->recoverypid > 0) kill (g->recoverypid, 9); - waitpid (g->pid, NULL, 0); + if (g->pid > 0) waitpid (g->pid, NULL, 0); if (g->recoverypid > 0) waitpid (g->recoverypid, NULL, 0); g->fd[0] = -1; g->fd[1] = -1; -- 1.7.4.1
On Mon, Mar 14, 2011 at 10:40:12PM +1100, Angus Salkeld wrote:
Signed-off-by: Angus Salkeld <asalkeld@redhat.com> --- src/launch.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/launch.c b/src/launch.c index e50985d..8de2857 100644 --- a/src/launch.c +++ b/src/launch.c @@ -769,7 +769,7 @@ launch_appliance (guestfs_h *g) } if (g->pid > 0) kill (g->pid, 9); if (g->recoverypid > 0) kill (g->recoverypid, 9); - waitpid (g->pid, NULL, 0); + if (g->pid > 0) waitpid (g->pid, NULL, 0); if (g->recoverypid > 0) waitpid (g->recoverypid, NULL, 0); g->fd[0] = -1; g->fd[1] = -1; -- 1.7.4.1
ACK. I'll push this in a moment. Note that for any changes that you would like added to the RHEL 6.2 package, you must file a bug: https://bugzilla.redhat.com/enter_bug.cgi?component=libguestfs&product=Red%20Hat%20Enterprise%20Linux%206 Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
On Mon, Mar 14, 2011 at 11:46:21AM +0000, Richard W.M. Jones wrote:
On Mon, Mar 14, 2011 at 10:40:12PM +1100, Angus Salkeld wrote:
Signed-off-by: Angus Salkeld <asalkeld@redhat.com> --- src/launch.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/launch.c b/src/launch.c index e50985d..8de2857 100644 --- a/src/launch.c +++ b/src/launch.c @@ -769,7 +769,7 @@ launch_appliance (guestfs_h *g) } if (g->pid > 0) kill (g->pid, 9); if (g->recoverypid > 0) kill (g->recoverypid, 9); - waitpid (g->pid, NULL, 0); + if (g->pid > 0) waitpid (g->pid, NULL, 0); if (g->recoverypid > 0) waitpid (g->recoverypid, NULL, 0); g->fd[0] = -1; g->fd[1] = -1; -- 1.7.4.1
ACK. I'll push this in a moment.
Note that for any changes that you would like added to the RHEL 6.2 package, you must file a bug: https://bugzilla.redhat.com/enter_bug.cgi?component=libguestfs&product=Red%20Hat%20Enterprise%20Linux%206
I finally managed to build and test this. I missed a simerlar change in src/guestfs.c So we need another little patch (attached). -Angus
Rich.
-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
On Tue, Mar 15, 2011 at 10:52:33PM +1100, Angus Salkeld wrote:
On Mon, Mar 14, 2011 at 11:46:21AM +0000, Richard W.M. Jones wrote:
On Mon, Mar 14, 2011 at 10:40:12PM +1100, Angus Salkeld wrote:
Signed-off-by: Angus Salkeld <asalkeld@redhat.com> --- src/launch.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/launch.c b/src/launch.c index e50985d..8de2857 100644 --- a/src/launch.c +++ b/src/launch.c @@ -769,7 +769,7 @@ launch_appliance (guestfs_h *g) } if (g->pid > 0) kill (g->pid, 9); if (g->recoverypid > 0) kill (g->recoverypid, 9); - waitpid (g->pid, NULL, 0); + if (g->pid > 0) waitpid (g->pid, NULL, 0); if (g->recoverypid > 0) waitpid (g->recoverypid, NULL, 0); g->fd[0] = -1; g->fd[1] = -1; -- 1.7.4.1
ACK. I'll push this in a moment.
Note that for any changes that you would like added to the RHEL 6.2 package, you must file a bug: https://bugzilla.redhat.com/enter_bug.cgi?component=libguestfs&product=Red%20Hat%20Enterprise%20Linux%206
I finally managed to build and test this. I missed a simerlar change in src/guestfs.c So we need another little patch (attached).
Thanks, this has been pushed upstream. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
participants (2)
-
Angus Salkeld -
Richard W.M. Jones