
On Tue, May 25, 2010 at 11:56:15AM -0600, Eric Blake wrote:
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=...").
I was going to say 'the same rules as execvp' but I now realize there is no execvp variant that also accepts a char * argv + char *env[]. Only a va_args variant. And execve doesn't do path resolution. It doesn't hugely matter which semantics we have - which do you suggest.
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"?
One arg. The two arg case is dealt with by just calling AddArg() twice.
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.
What does execve() do if env[] has the same name twice ? We'll just be delegating to that
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?
AFAIK, none of our current usage requires it, but if the conversion of existing code requires it, we can adapt.
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.
We already have a convenient virPidFile(path, name) function, so probably isn't required here.
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?
Correct, FD 7 in the parent will appear as FD 0 in the child. No FD 7 will be visible in the child.
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?
I'm intending to declare that the caller must close these FDs when it decides best.
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?
The impl of virCommandRun() calls out to virProcessIO which uses select() to wait on the stdin+out+err, so we can avoid blocking. But this reminds me that I should set the FDs O_NONBLOCK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|