Re: [libvirt] [PATCH 08/10] Introduce new APIs for spawning processes
On Wed, Nov 17, 2010 at 09:29:00PM -0700, Eric Blake wrote:
From: Daniel P. Berrange <berrange(a)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 :|