[libvirt] [PATCH 1/8] Allow handshake with child process during startup

[Eric Blake]

On 05/11/2011 03:33 AM, Daniel P. Berrange wrote:
> Allow the parent process to perform a bi-directional handshake
> with the child process during fork/exec. The child process
> will fork and do its initial setup. Immediately prior to the
> exec(), it will stop & wait for a handshake from the parent
> process. The parent process will spawn the child and wait
> until the child reaches the handshake point. It will do
> whatever extra setup work is required, before signalling the
> child to continue.
> 
> The implementation of this is done using two pairs of blocking
> pipes. The first pair is used to block the parent, until the
> child writes a single byte. Then the second pair pair is used
> to block the child, until the parent confirms with another
> single byte.
> 
> * src/util/command.c, src/util/command.h,
>   src/libvirt_private.syms: Add APIs to perform a handshake
> ---
>  src/libvirt_private.syms |    3 +
>  src/util/command.c       |  161 +++++++++++++++++++++++++++++++++++++++++++++-
>  src/util/command.h       |   22 ++++++
>  3 files changed, 185 insertions(+), 1 deletions(-)

Hopefully there aren't too many rebase issues if Cole's virCommand
cleanup series goes in first.

> @@ -1255,6 +1315,10 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
>              FD_CLR(i, &cmd->transfer);
>          }
>      }
> +    if (cmd->handshake) {
> +        VIR_FORCE_CLOSE(cmd->handshakeWait[1]);
> +        VIR_FORCE_CLOSE(cmd->handshakeNotify[0]);
> +    }

You don't need this hunk if you use virCommandTransferFD below...

> @@ -1395,6 +1459,94 @@ virCommandAbort(virCommandPtr cmd ATTRIBUTE_UNUSED)
>  }
>  #endif
>  
> +
> +void virCommandRequireHandshake(virCommandPtr cmd)
> +{
> +    if (!cmd || cmd->has_error)
> +        return;

Avoid clobbering existing fds and causing an fd leak, by adding:

if (cmd->handshake) {
    cmd->has_error = -1;
    VIR_DEBUG("cannot require handshake twice");
    return;
}

> +
> +    if (pipe(cmd->handshakeWait) < 0) {
> +        cmd->has_error = errno;
> +        return;
> +    }
> +    if (pipe(cmd->handshakeNotify) < 0) {
> +        VIR_FORCE_CLOSE(cmd->handshakeWait[0]);
> +        VIR_FORCE_CLOSE(cmd->handshakeWait[1]);
> +        cmd->has_error = errno;
> +        return;
> +    }
> +
> +    VIR_DEBUG("Transfer handshake wait=%d notify=%d",
> +              cmd->handshakeWait[1], cmd->handshakeNotify[0]);
> +    virCommandPreserveFD(cmd, cmd->handshakeWait[1]);
> +    virCommandPreserveFD(cmd, cmd->handshakeNotify[0]);

...here's where to use virCommandTransferFD, for slightly less bookkeeping.

> +int virCommandHandshakeWait(virCommandPtr cmd)
> +{
> +    char c;
> +    int rv;
> +    if (!cmd ||cmd->has_error == ENOMEM) {
> +        virReportOOMError();
> +        return -1;
> +    }
> +    if (cmd->has_error) {

Change to this, to avoid calling saferead on an invalid fd if no one
called virCommandRequireHandshake:

if (cmd->has_error || !cmd->handshake)

Also, on completion you either need to set cmd->handshake = false or
call VIR_CLOSE the fd on completion; if you do the latter, then on entry
you should check that the fd is not -1, so that we ensure no one calls
this method twice for a single child process.

> +    if (c != '1') {
> +        char *msg;
> +        ssize_t len;
> +        if (VIR_ALLOC_N(msg, 1024) < 0) {
> +            virReportOOMError();

Should we stack-allocate this, to minimize the chance of a malloc
failure while reporting the child's failure?

> +
> +int virCommandHandshakeNotify(virCommandPtr cmd)
> +{
> +    char c = '1';
> +    if (!cmd ||cmd->has_error == ENOMEM) {
> +        virReportOOMError();
> +        return -1;
> +    }
> +    if (cmd->has_error) {

if (cmd->has_error || !cmd->handshake)

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110512/e679af16/attachment-0001.sig>