Re: [libvirt] [PATCHv2 3/8] Introduce new APIs for spawning processes

From: Daniel P. Berrange <berrange(a)redhat.com&gt; This introduces a new set of APIs in src/util/command.h to use for invoking commands. This is intended to replace all current usage of virRun and virExec variants, with a more flexible and less error prone API. * src/util/command.c: New file. * src/util/command.h: New header. * src/Makefile.am (UTIL_SOURCES): Build it. * src/libvirt_private.syms: Export symbols internally. * tests/commandtest.c: New test. * tests/Makefile.am (check_PROGRAMS): Run it. * tests/commandhelper.c: Auxiliary program. * tests/commanddata/test2.log - test15.log: New expected outputs. * cfg.mk (useless_free_options): Add virCommandFree. * po/POTFILES.in: New translation. * .x-sc_avoid_write: Add exemption. * tests/.gitignore: Ignore new built file. --- v2: add virCommandTransferFD, virCommandToString, virCommandAddArgFormat, virCommandNewArgList, virCommandWriteArgLog, virCommandNonblockingFDs. Fix virCommandRunAsync and virCommandFree to free transfered FDs. Add a bit more test exposure.

diff --git a/cfg.mk b/cfg.mk index dea8301..6312632 100644 --- a/cfg.mk +++ b/cfg.mk @@ -77,6 +77,7 @@ useless_free_options =                                \   --name=virCapabilitiesFreeHostNUMACell       \   --name=virCapabilitiesFreeMachines           \   --name=virCgroupFree                         \ +  --name=virCommandFree                                \   --name=virConfFreeList                       \   --name=virConfFreeValue                      \   --name=virDomainChrDefFree                   \

diff --git a/src/util/command.c b/src/util/command.c new file mode 100644 index 0000000..948a957 --- /dev/null +++ b/src/util/command.c

+/* + * Create a new command with a NULL terminated + * set of args, taking binary from argv[0] + */

+virCommandPtr +virCommandNewArgs(const char *const*args)

+ +/* + * Capture the child's stdout to a string buffer + */ +void +virCommandSetOutputBuffer(virCommandPtr cmd, char **outbuf) +{ +    if (!cmd || cmd->has_error) +        return; + +    if (cmd->outfd != -1) {

+        cmd->has_error = -1; +        VIR_DEBUG0("cannot specify output twice"); +        return; +    } + +    cmd->outbuf = outbuf; +    cmd->outfdptr = &cmd->outfd; +} + + +/* + * Capture the child's stderr to a string buffer + */ +void +virCommandSetErrorBuffer(virCommandPtr cmd, char **errbuf) +{ +    if (!cmd || cmd->has_error) +        return; + +    if (cmd->errfd != -1) {

+        cmd->has_error = -1; +        VIR_DEBUG0("cannot specify stderr twice"); +        return; +    } + +    cmd->errbuf = errbuf; +    cmd->errfdptr = &cmd->errfd; +} + + +/* + * Attach a file descriptor to the child's stdin + */ +void +virCommandSetInputFD(virCommandPtr cmd, int infd) +{ +    if (!cmd || cmd->has_error) +        return; + +    if (infd < 0 || cmd->inbuf) {

+        cmd->has_error = -1; +        VIR_DEBUG0("cannot specify input twice"); +        return; +    } + +    cmd->infd = infd; +} + + +/* + * Attach a file descriptor to the child's stdout + */ +void +virCommandSetOutputFD(virCommandPtr cmd, int *outfd) +{ +    if (!cmd || cmd->has_error) +        return; + +    if (!outfd || cmd->outfd != -1) {

+        cmd->has_error = -1; +        VIR_DEBUG0("cannot specify output twice"); +        return; +    } + +    cmd->outfdptr = outfd; +} + + +/* + * Attach a file descriptor to the child's stderr + */ +void +virCommandSetErrorFD(virCommandPtr cmd, int *errfd) +{ +    if (!cmd || cmd->has_error) +        return; + +    if (!errfd || cmd->errfd != -1) {

+        cmd->has_error = -1; +        VIR_DEBUG0("cannot specify stderr twice"); +        return; +    } + +    cmd->errfdptr = errfd; +}

+ +/* + * Call after adding all arguments and environment settings, but before + * Run/RunAsync, to immediately output the environment and arguments of + * cmd to logfd.  If virCommandRun cannot succeed (because of an + * out-of-memory condition while building cmd), nothing will be logged. + */ +void +virCommandWriteArgLog(virCommandPtr cmd, int logfd) +{ +    int ioError = 0; +    size_t i; + +    /* Any errors will be reported later by virCommandRun, which means +     * no command will be run, so there is nothing to log. */ +    if (!cmd || cmd->has_error) +        return; + +    for (i = 0 ; i < cmd->nenv ; i++) { +        if (safewrite(logfd, cmd->env[i], strlen(cmd->env[i])) < 0) +            ioError = errno; +        if (safewrite(logfd, " ", 1) < 0) +            ioError = errno; +    } +    for (i = 0 ; i < cmd->nargs ; i++) { +        if (safewrite(logfd, cmd->args[i], strlen(cmd->args[i])) < 0) +            ioError = errno; +        if (safewrite(logfd, i == cmd->nargs - 1 ? "\n" : " ", 1) < 0) +            ioError = errno; +    }

+    if (ioError) { +        char ebuf[1024]; +        VIR_WARN("Unable to write command %s args to logfile: %s", +                 cmd->args[0], virStrerror(ioError, ebuf, sizeof ebuf)); +    } +}

+/* + * Manage input and output to the child process. + */ +static int +virCommandProcessIO(virCommandPtr cmd) +{ +    int infd = -1, outfd = -1, errfd = -1; +    size_t inlen = 0, outlen = 0, errlen = 0; +    size_t inoff = 0; + +    /* With an input buffer, feed data to child +     * via pipe */ +    if (cmd->inbuf) { +        inlen = strlen(cmd->inbuf); +        infd = cmd->inpipe; +    } + +    /* With out/err buffer, the outfd/errfd +     * have been filled with an FD for us */ +    if (cmd->outbuf) +        outfd = cmd->outfd; +    if (cmd->errbuf) +        errfd = cmd->errfd; +

+/* + * Run the command and wait for completion. + * Returns -1 on any error executing the + * command. Returns 0 if the command executed, + * with the exit status set + */ +int +virCommandRun(virCommandPtr cmd, int *exitstatus) +{ +    int ret = 0; +    char *outbuf = NULL; +    char *errbuf = NULL; +    int infd[2]; + +    if (!cmd || cmd->has_error == -1) { +        virCommandError(VIR_ERR_INTERNAL_ERROR, "%s", +                        _("invalid use of command API")); +        return -1; +    } +    if (cmd->has_error == ENOMEM) { +        virReportOOMError(); +        return -1; +    } + +    /* If we have an input buffer, we need +     * a pipe to feed the data to the child */ +    if (cmd->inbuf) { +        if (pipe(infd) < 0) { +            virReportSystemError(errno, "%s", +                                 _("unable to open pipe")); +            return -1; +        } +        cmd->infd = infd[0]; +        cmd->inpipe = infd[1]; +    } + +    if (virCommandRunAsync(cmd, NULL) < 0) { +        if (cmd->inbuf) { +            int tmpfd = infd[0]; +            if (VIR_CLOSE(infd[0]) < 0) +                VIR_DEBUG("ignoring failed close on fd %d", tmpfd); +            tmpfd = infd[1]; +            if (VIR_CLOSE(infd[1]) < 0) +                VIR_DEBUG("ignoring failed close on fd %d", tmpfd); +        } +        return -1; +    } + +    /* If caller hasn't requested capture of +     * stdout/err, then capture it ourselves +     * so we can log it +     */ +    if (!cmd->outbuf && +        !cmd->outfdptr) { +        cmd->outfd = -1; +        cmd->outfdptr = &cmd->outfd; +        cmd->outbuf = &outbuf; +    } +    if (!cmd->errbuf && +        !cmd->errfdptr) { +        cmd->errfd = -1; +        cmd->errfdptr = &cmd->errfd; +        cmd->errbuf = &errbuf; +    } + +    if (cmd->inbuf || cmd->outbuf || cmd->errbuf) +        ret = virCommandProcessIO(cmd);

diff --git a/tests/commandtest.c b/tests/commandtest.c new file mode 100644 index 0000000..46d6ae5 --- /dev/null +++ b/tests/commandtest.c

+ +#ifndef __linux__