Re: [libvirt] [PATCH 3/7] Add helper function virExecDaemonize

[Cole Robinson <crobinso redhat com>]

Daniel P. Berrange wrote:
> On Tue, Apr 28, 2009 at 11:31:53AM -0400, Cole Robinson wrote:
>> Wraps __virExec with the VIR_EXEC_DAEMON flag. Waits on the intermediate
>> process to ensure we don't end up with any zombies, and differentiates between
>> original process errors and intermediate process errors.
>> +int virExecDaemonize(virConnectPtr conn,
>> +                     const char *const*argv,
>> +                     const char *const*envp,
>> +                     const fd_set *keepfd,
>> +                     pid_t *retpid,
>> +                     int infd, int *outfd, int *errfd,
>> +                     int flags,
>> +                     virExecHook hook,
>> +                     void *data) {
>> +    int ret;
>> +    int childstat = 0;
>> +
>> +    ret = virExecWithHook(conn, argv, envp, keepfd, retpid,
>> +                          infd, outfd, errfd,
>> +                          flags |= VIR_EXEC_DAEMON,
>> +                          hook, data);
>> +
>> +    /* Wait for intermediate process to exit */
>> +    while (waitpid(*retpid, &childstat, 0) == -1 &&
>> +                   errno == EINTR);
> 
> I think we should only call waitpid() if 'ret == 0', because if ret is
> not 0, *retpid won't have been set and will thus contain random garbage.
> 

Good catch. I've resent the series with this and your others comments
addressed.

Thanks,
Cole