[libvirt] [PATCH 12/23] Add JSON serialization of virLockSpacePtr objects for process re-exec()

[Daniel P. Berrange]

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 at redhat.com>
> >>
> >> 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 at redhat.com>
> >> ---
> > 
> > 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 :|