12:56 p.m.
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org