On Wed, Nov 17, 2010 at 09:29:00PM -0700, Eric Blake wrote:
> From: Daniel P. Berrange <berrange@xxxxxxxxxx>
>
> 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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list