On Wed, Nov 17, 2010 at 09:29:00PM -0700, Eric Blake wrote:
From: Daniel P. Berrange <berrange@redhat.com>
This introduces a new set of APIs in src/util/command.h to use for invoking commands. This is intended to replace all current usage of virRun and virExec variants, with a more flexible and less error prone API.
* src/util/command.c: New file. * src/util/command.h: New header. * src/Makefile.am (UTIL_SOURCES): Build it. * src/libvirt_private.syms: Export symbols internally. * tests/commandtest.c: New test. * tests/Makefile.am (check_PROGRAMS): Run it. * tests/commandhelper.c: Auxiliary program. * tests/commanddata/test2.log - test15.log: New expected outputs. * cfg.mk (useless_free_options): Add virCommandFree. * po/POTFILES.in: New translation. * .x-sc_avoid_write: Add exemption. * tests/.gitignore: Ignore new built file.
+struct _virCommand { + int has_error; /* ENOMEM on allocation failure, -1 for anything else. */ + + char **args; + size_t nargs; + size_t maxargs; + + char **env; + size_t nenv; + size_t maxenv; + + char *pwd; + + /* XXX Use int[] if we ever need to support more than FD_SETSIZE fd's. */ + fd_set preserve; + unsigned int flags; + + char *inbuf; + char **outbuf; + char **errbuf; + + int infd; + int inpipe; + int outfd; + int errfd; + int *outfdptr; + int *errfdptr; + + virExecHook hook; + void *opaque; + + pid_t pid; + char *pidfile; +}; +/* + * Release all resources + */ +void +virCommandFree(virCommandPtr cmd) +{ + int i; + if (!cmd) + return; + + VIR_FORCE_CLOSE(cmd->outfd); + VIR_FORCE_CLOSE(cmd->errfd); + + for (i = 0 ; i < cmd->nargs ; i++) + VIR_FREE(cmd->args[i]); + VIR_FREE(cmd->args); + + for (i = 0 ; i < cmd->nenv ; i++) + VIR_FREE(cmd->env[i]); + VIR_FREE(cmd->env); + + VIR_FREE(cmd->pwd); + + VIR_FREE(cmd->pidfile); + + VIR_FREE(cmd); +}
My code forgot to ever close() the fds in cmd->preserve. We definitely need todo it in virCommandFree(), but there's a small argument to say we should also do it in virCommandRun/virCommandRunAsync so that if the caller keeps the virCommandPtr alive for a long time, we don't have the open FDs. It would also be useful to have a generic API for logging info about the command to an FD (to let us remove that logging code from UML and QEMU & LXC drivers). eg +void virCommandWriteArgLog(virCommandPtr cmd, int logfd) +{ + int ioError = 0; + int i; + + for (i = 0 ; i < cmd->nenv ; i++) { + if (safewrite(logfd, cmd->env[i], strlen(cmd->env[i])) < 0) + ioError = errno; + if (safewrite(logfd, " ", 1) < 0) + ioError = errno; + } + for (i = 0 ; i < cmd->nargs ; i++) { + if (safewrite(logfd, cmd->args[i], strlen(cmd->args[i])) < 0) + ioError = errno; + if (safewrite(logfd, " ", 1) < 0) + ioError = errno; + } + + if (ioError) { + char ebuf[1024]; + VIR_WARN("Unable to write command %s args to logfile: %s", + cmd->args[0], virStrerror(ioError, ebuf, sizeof ebuf)); + } +} Regards, 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 :|