[libvirt] [PATCH 2/3] Docs for usage of new command APIs
Eric Blake
eblake at redhat.com
Tue May 25 21:14:18 UTC 2010
On 05/25/2010 07:24 AM, Daniel P. Berrange wrote:
> ---
> docs/Makefile.am | 12 ++++++++++--
> docs/internals.html.in | 9 +++++++++
> docs/sitemap.html.in | 4 ++++
> 3 files changed, 23 insertions(+), 2 deletions(-)
>
> +++ b/docs/Makefile.am
> @@ -53,7 +53,7 @@ gif = \
> architecture.gif \
> node.gif
>
> -dot_html_in = $(wildcard *.html.in)
> +dot_html_in = $(wildcard *.html.in) $(wildcard internals/*.html.in)
> dot_html = $(dot_html_in:%.html.in=%.html)
Note to myself: If I ever get time to revisit my VPATH patches, I've got
a merge conflict in this area.
Jim already pointed out the new file omission, but given your first
message had the text-ified version, here's some spelling fixes:
> Spawning processes / commands from libvirt drivers
>
> This page describes the usage of libvirt APIs for spawning processes /
> commands from libvirt drivers. All code is required to use these APIs
>
> Problems with standard POSIX APIs
>
> The POSIX specification includes a number of APIs for spawning processes /
> commands, but they suffer from a number of flaws
>
> * fork+exec: The lowest & most flexible level, but very hard to use
> correctly / safely. It is easy to leak file descriptors, have
> unexpected signal handler behaviour and not handle edge cases
Also, not portable to mingw.
> * system: Convenient if you don't care about capturing command output,
> but has the serious downside that the command string is interpreted by
> the shell. This makes it very dangerous to use, because improperly
> validated user input can lead to exploits via shell meta characters.
* popen: Similar problems to system; also has issues affecting signal
handling related to other child processes.
> * posix_spawn: A half-way house between simplicity of system() and the
> flexibility of fork+exec. It does not allow for a couple of important
> features though, such as running a hook between the fork+exec stage,
> or closing all open file descriptors.
>
> Due to the problems mentioned with each of these, libvirt driver code must
> not use any of the above APIs. Historically libvirt provided a higher
> level API known as virExec. This was wrapper around fork+exec, in a
> similar style to posix_spawn, but with a few more features.
I didn't see anything in patch 1/3 that deprecated virExec in the public
headers. Is it worth adding conditional deprecation (guarded by a macro
defined only in command.c), to make sure it is not called anywhere
except in the command.c implementation?
>
> This wrapper still suffered from a number of problems. Handling command
> cleanup via waitpid() is overly complex & error prone for most usage.
> Building up the argv[] + env[] string arrays is quite cumbersome and error
> prone, particularly wrt memory leak / OOM handling.
>
> The libvirt command execution API
>
> There is now a high level API that provides a safe and flexible way to
> spawn commands, which prevents the most common errors & is easy to code
s/&/and/
> against. This code is provided in the src/util/command.h header which can
> be imported using #include "command.h"
>
> 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.
>
> virCommandPtr cmd = virCommandNew("/usr/bin/dnsmasq");
>
>
> There is no need to check for allocation failure after virCommandNew. This
> will be detected and reported at a later time.
>
> Adding arguments to the command
>
> There are a number of APIs for adding arguments to a command. To add a
> direct string arg
>
> virCommandAddArg(cmd, "-strict-order");
>
>
> 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");
>
>
> To add a entire NULL terminated array of arguments in one go
>
> const char *const args[] = {
> "--strict-order", "--except-interface",
> "lo", "--domain", "localdomain", NULL
> };
> virCommandAddArgSet(cmd, args);
>
>
> This latter option can also be used at time of initial construction of the
> virCommandPtr object
>
> const char *const args[] = {
> "--strict-order", "--except-interface",
> "lo", "--domain", "localdomain", NULL
> };
> virCommandPtr cmd = virCommandNewArgs(cmd, args);
>
>
> 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");
>
>
> To define an environment variable in the child with an separate key /
> value:
>
> virCommandAddEnvPair(cmd, "TERM", "xterm");
>
>
> If the key/value pair is pre-formatted in the right format, it can be set
> directly
>
> virCommandAddEnvString(cmd, "TERM=xterm");
>
>
> Miscellaneous other options
>
> Normally the spawned command will retain the current process as its
> parent. If the current process dies, the child will then (usually) be
> terminated too. If this cleanup is not desired, then the command should be
> marked as daemonized:
>
> virCommandDaemonize(cmd);
>
>
> 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
s/damonized/daemonized/
> of the real command is required then a pidfile can be requested
>
> virCommandSetPidFile(cmd, "/var/run/dnsmasq.pid");
>
>
> This PID file is guaranteed to be written before the intermediate process
> exits.
>
> Reducing command privileges
>
> Normally a command will inherit all privileges of the current process. To
> restrict what a command can do, it is possible to request that all its
> capabilities are cleared. With this done it will only be able to access
> resources for which it has explicit DAC permissions
>
> virCommandClearCaps(cmd);
>
>
> 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
s/neccessary/necessary/
> map a file handle. To attach file descriptor 7 in the current process to
> stdin in the child:
>
> virCommandSetInputFD(cmd, 7);
>
>
> Equivalently to redirect stdout or stderr in the child, pass in a pointer
> to the desired handle
>
> int outfd = open("out.log", "w+");
> int errfd = open("err.log", "w+");
> virCommandSetOutputFD(cmd, &outfd);
> virCommandSetErrorFD(cmd, &errfd);
>
>
> Alternatively it is possible to request that a pipe be created to fetch
> stdout/err in the parent, by initializing the FD to -1.
>
> int outfd = -1;
> int errfd = -1
> 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.
>
> Feeding & capturing strings to/from the child
>
> Often dealing with file handles for stdin/out/err is unneccessarily
s/unneccessarily/unnecessarily/
> 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);
>
>
> Similarly it is possible to request that the child's stdout/err be
> redirected into a string buffer
>
> char *output = NULL, *errors = NULL;
> virCommandSetOutputBuffer(cmd, &output);
> virCommandSetErrorBuffer(cmd, &errors);
>
>
> Once the command has finished executing, these buffers will contain the
> output. It is the callers responsibility to free these buffers.
>
> Running commands synchronously
>
> For most commands, the desired behaviour is to spawn the command, wait for
/me decides not to even bother with the UK vs. US issue.
> it to complete & exit and then check that its exit status is zero.
>
> if (virCommandRun(cmd, NULL) < 0)
> return -1;
>
>
> Note: if the command has been daemonized this will only block & wait for
> the intermediate process, not the real command. virCommandRun will report
> on any errors that have occured upon this point with all previous API
s/occured/occurred/
> calls. If the command fails to run, or exits with non-zero status an error
> will be reported via normal libvirt error infrastructure. If a non-zero
> exit status can represent a success condition, it is possible to request
> the exit status and perform that check manually instead of letting
> virCommandRun raise the error
>
> int status;
> if (virCommandRun(cmd, &status) < 0)
> return -1;
>
> if (WEXITSTATUS(status) ...) {
> ...do stuff...
> }
>
>
> Running commands asynchronously
>
> In certain complex scenarios, particularly special I/O handling is
> required for the child's stdin/err/out it will be neccessary to run the
s/neccessary/necessary/
> command asynchronously and wait for completion separately.
>
> pid_t pid;
> if (virCommandRunAsync(cmd, &pid) < 0)
> return -1;
>
> ... do something while pid is running ...
>
> int status;
> if (virCommandWait(cmd, &status) < 0)
> return -1;
>
> if (WEXITSTATUS(status)...) {
> ..do stuff..
> }
>
>
> As with virCommandRun, the status arg for virCommandWait can be omitted,
> in which case it will validate that exit status is zero and raise an error
> if not.
>
> Releasing resources
>
> Once the command has been executed, or if execution has been abandoned, it
> is neccessary to release resources associated with the virCommandPtr
> object. This is done with:
>
> virCommandFree(cmd);
>
>
> There is no need to check if cmd is NULL before calling virCommandFree.
> This scenario is handled automatically. If the command is still running,
> it will be forcably killed and cleaned up (via waitpid).
s/forcably/forcibly/
>
> 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);
>
> 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.
>
Also, see my comments to 0/3 regarding semantic issues (this review only
did spelling/grammar issues).
--
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/541e0f97/attachment-0001.sig>
More information about the libvir-list
mailing list