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

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(a)redhat.com&gt; >> >> 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(a)redhat.com&gt; >> --- > > 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.