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

[Daniel P. Berrange]

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 :|