Re: [libvirt] [PATCH 13/23] Add JSON serialization of virNetSocketPtr objects for process re-exec()
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(a)redhat.com>
>>
>> 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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)