[libvirt] [PATCH 0/3 RFC] Demonstrating a new API for spawning processes

[Eric Blake]

On 05/25/2010 07:24 AM, Daniel P. Berrange wrote:
> We have had great success with our APIs for memory allocation and
> string buffer management, removing most of the potential for incorrect
> API usage, thus avoiding many common errors. It is time todo the same
> for command execution.

Yes, this makes a good addition.

> 
>   Defining commands in libvirt
> 
>    The first step is to declare what command is to be executed. The command
>    name can be either a fully qualified path, or a bare command name. In the
>    latter case it will be resolved wrt the $PATH environment variable.

The $PATH of the parent process, or the $PATH of the environment passed
to the child process?  That can make a subtle difference, if one uses
virCommandAddEnvString(cmd, "PATH=...").

> 
>    If an argument takes an attached value of the form -arg=val, then this can
>    be done using
> 
>        virCommandAddArgPair(cmd, "--conf-file", "/etc/dnsmasq.conf");

Does this create the two arguments "--conf-file /etc/dnsmasq.conf" or
the one argument "--conf-file=/etc/dnsmasq.conf"?

>   Setting up the environment
> 
>    By default a command will inherit all environment variables from the
>    current process. Generally this is not desirable and a customized
>    environment will be more suitable. Any customization done via the
>    following APIs will prevent inheritance of any existing environment
>    variables unless explicitly allowed. The first step is usually to pass
>    through a small number of variables from the current process.
> 
>        virCommandAddEnvPassCommon(cmd);
> 
> 
>    This has now set up a clean environment for the child, passing through
>    PATH, LD_PRELOAD, LD_LIBRARY_PATH, HOME, USER, LOGNAME and TMPDIR.
>    Furthermore it will explicitly set LC_ALL=C to avoid unexpected
>    localization of command output. Further variables can be passed through
>    from parent explicitly:
> 
>        virCommandAddEnvPass(cmd, "DISPLAY");
>        virCommandAddEnvPass(cmd, "XAUTHORITY");

If the same env-var is added more than once, are we guaranteeing that
the last one wins?  In other words, it should be easy to call
virCommandAddEnvPassCommon(cmd) && virCommandAddEnvString(cmd,
"PATH=...") to override just PATH.

Should there be an easy way to specify that a particular child process
should keep the localization settings of the parent, rather than the
LC_ALL=C of virCommandAddEnvPassCommon?

> 
>    When daemonizing a command, the PID visible from the caller will be that
>    of the intermediate process, not the actual damonized command. If the PID
>    of the real command is required then a pidfile can be requested
> 
>        virCommandSetPidFile(cmd, "/var/run/dnsmasq.pid");

Is this worth a printf-style varargs call, to make it easier to cobble
together components?  Perhaps like:
virCommandSetPidFile(cmd, "%s/%s.pid", LOCAL_STATE_DIR, "dnsmasq")

On the other hand, it's relatively easy to build up a string using
virBuffer APIs, so we might as well keep the virCommand API simple.

>   Managing file handles
> 
>    To prevent unintended resource leaks to child processes, all open file
>    handles will be closed in the child, and its stdin/out/err all connected
>    to /dev/null. It is possible to allow an open file handle to be passed
>    into the child:
> 
>        virCommandPreserveFD(cmd, 5);
> 
> 
>    With this file descriptor 5 in the current process remains open as file
>    descriptor 5 in the child. For stdin/out/err it is usually neccessary to
>    map a file handle. To attach file descriptor 7 in the current process to
>    stdin in the child:
> 
>        virCommandSetInputFD(cmd, 7);

Does the child see fd 7 closed in this case?

>        virCommandSetOutputFD(cmd, &outfd);
>        virCommandSetErrorFD(cmd, &errfd);
> 
> 
>    Once the command is running, outfd and errfd will be initialized with
>    valid file handles that can be read from.

Any restrictions on when (or even if) the parent process may/must call
close(outfd)?  In other words, must the parent's side of the output fd
remain open until the exit status of the child has been collected, and
does collecting the child's status automatically close the parent's side?

> 
>   Feeding & capturing strings to/from the child
> 
>    Often dealing with file handles for stdin/out/err is unneccessarily
>    complex. It is possible to specify a string buffer to act as the data
>    source for the child's stdin
> 
>        const char *input = "Hello World\n";
>        virCommandSetInputBuffer(cmd, input);

Any limitations to be aware of to avoid I/O deadlock when set up as a
bi-directional pipe to the child?

> Complete examples
> 
>    This shows a complete example usage of the APIs roughly using the libvirt
>    source src/util/hooks.c
> 
>      int runhook(const char *drvstr, const char *id,
>                  const char *opstr, const char *subopstr,
>                  const char *extra) {
>        int ret;
>        char *path;
>        virCommandPtr cmd;
> 
>        ret = virBuildPath(&path, LIBVIRT_HOOK_DIR, drvstr);
>        if ((ret < 0) || (path == NULL)) {
>            virHookReportError(VIR_ERR_INTERNAL_ERROR,
>                               _("Failed to build path for %s hook"),
>                               drvstr);
>            return -1;
>        }
> 
>        cmd = virCommandNew(path);
>        VIR_FREE(path);
> 
>        virCommandAddEnvPassCommon(cmd);
> 
>        virCommandAddArg(cmd, id);
>        virCommandAddArg(cmd, opstr);
>        virCommandAddArg(cmd, subopstr);
>        virCommandAddArg(cmd, extra);
> 
>        virCommandSetInputBuffer(cmd, input);

Where is input declared?

> 
>        ret = virCommandRun(cmd, NULL);
> 
>        virCommandFree(cmd);
> 
>        return ret;
> 
> 
>    In this example, the command is being run synchronously. A pre-formatted
>    string is being fed to the command as its stdin. The command takes four
>    arguments, and has a minimal set of environment variables passed down. In
>    this example, the code does not require any error checking. All errors are
>    reported by the virCommandRun method, and the exit status from this is
>    returned to the caller to handle as desired.

Overall, this looks like a nice addition.  More specific comments to the
various patches themselves.

-- 
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/20100525/d1c4602a/attachment-0001.sig>