On Fri, Aug 17, 2012 at 11:25:37AM -0600, Eric Blake wrote: > On 08/16/2012 05:07 PM, Eric Blake wrote: > > On 08/09/2012 09:20 AM, Daniel P. Berrange wrote: > >> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx> > >> > >> Add two new APIs virLockSpaceNewPostExecRestart and > >> virLockSpacePreExecRestart which allow a virLockSpacePtr > >> object to be created from a JSON object and saved to a > >> JSON object, for the purposes of re-exec'ing a process. > >> > >> As well as saving the state in JSON format, the second > >> method will disable the O_CLOEXEC flag so that the open > >> file descriptors are preserved across the process re-exec() > >> > >> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx> > >> --- > > > > Is virLockSpacePreExecRestart called in the parent prior to forking > > (mostly good, with one caveat) or in the child between fork and exec > > (bad, since you malloc and do a lot of other non-async-safe stuff)? If > > the latter, we risk deadlock; if the former, then you have at least one > > bug... > > Before you worry about responding to this, read my comments on 21/23. I > think I've managed to convince myself that if these functions are only > ever called from virtlockd (and _not_ from libvirt.so or libvirtd), then > the fact that virtlockd is so dedicated-purpose as to never spawn a > helper child means that you don't have to worry about FD_CLOEXEC races > to arbitrary children. If you agree with my analysis, then this patch > (and several like it where I complained about CLOEXEC races) should be > okay as-is. It is not so much that it will only be called from virtlockd, but rather that this must only be called from a state where the process is basically idle. The intent is that the process has exit'd its event loop, and has stopped all I/O threads, etc. Basically the intent is that the process call this method (and its similar friends), and then immediately execve() itself. It should not be doing anything else in parallel with this. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list