On 08/21/2012 09:48 AM, Daniel P. Berrange wrote:
> On Thu, Aug 16, 2012 at 05:16:50PM -0600, Eric Blake wrote:
>> On 08/09/2012 09:20 AM, Daniel P. Berrange wrote:
>>> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>>>
>>> Add two new APIs virNetSocketNewPostExecRestart and
>>> virNetSocketPreExecRestart which allow a virNetSocketPtr
>>> object to be created from a JSON object and saved to a
>>> JSON object, for the purpose 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()
>>
>> Same problem as 12/23 regarding _when_ you clear O_CLOEXEC.
>>
>>> + if (virJSONValueObjectGetNumberInt(object, "errfd", &errfd) < 0) {
>>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>> + _("Missing errfd data in JSON document"));
>>> + return NULL;
>>> + }
>>
>> Do you need to re-enable FD_CLOEXEC on fd and errfd at this point?
>
> In the scenario in which it is valid to call these APIs, the only
> thing the caller can do is to exit(), so I'd say we don't need to
> deal with re-enabling FD_CLOEXEC
Fair enough - I think I've managed to convince myself later in the
series, and your replies confirm, that this API is safe _for the
situation in which we are using it_; maybe a bit of documentation about
why we are doing things that are normally unsafe in multithreaded
programs that exec arbitrary children is in order (because although the
virtlockd program is multi-threaded, it does not exec arbitrary
children, and the one exec that it does do is at a safe point in
processing). I'm not sure if you are planning on submitting a rebase
(since there were other things I pointed out along the way that might
need fixing), but you have my ACK on the design.
--
Eric Blake eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list