[libvirt] PATCH: 5/5: Make all code use virExec
Daniel P. Berrange
berrange at redhat.com
Wed Aug 13 09:22:22 UTC 2008
This final patch switches over all places which do fork()/exec() to use the
new enhanced virExec() code. A fair amount of code is deleted, but that's
not the whole story - the new impl is more robust that most of the existing
code we're deleting here.
bridge.c | 51 +++-------------
proxy_internal.c | 25 +-------
qemu_conf.c | 168 ++++++++++++++++++++++--------------------------------
remote_internal.c | 88 ++++------------------------
4 files changed, 100 insertions(+), 232 deletions(-)
Daniel
diff -r 2591ebc40bd7 src/bridge.c
--- a/src/bridge.c Tue Aug 12 15:29:29 2008 +0100
+++ b/src/bridge.c Tue Aug 12 15:33:42 2008 +0100
@@ -46,6 +46,7 @@
#include "internal.h"
#include "memory.h"
+#include "util.h"
#define MAX_BRIDGE_ID 256
@@ -596,42 +597,6 @@
return brGetInetAddr(ctl, ifname, SIOCGIFNETMASK, addr, maxlen);
}
-static int
-brctlSpawn(char * const *argv)
-{
- pid_t pid, ret;
- int status;
- int null = -1;
-
- if ((null = open(_PATH_DEVNULL, O_RDONLY)) < 0)
- return errno;
-
- pid = fork();
- if (pid == -1) {
- int saved_errno = errno;
- close(null);
- return saved_errno;
- }
-
- if (pid == 0) { /* child */
- dup2(null, STDIN_FILENO);
- dup2(null, STDOUT_FILENO);
- dup2(null, STDERR_FILENO);
- close(null);
-
- execvp(argv[0], argv);
-
- _exit (1);
- }
-
- close(null);
-
- while ((ret = waitpid(pid, &status, 0) == -1) && errno == EINTR);
- if (ret == -1)
- return errno;
-
- return (WIFEXITED(status) && WEXITSTATUS(status) == 0) ? 0 : EINVAL;
-}
/**
* brSetForwardDelay:
@@ -649,7 +614,7 @@
const char *bridge,
int delay)
{
- char **argv;
+ const char **argv;
int retval = ENOMEM;
int n;
char delayStr[30];
@@ -680,7 +645,10 @@
argv[n++] = NULL;
- retval = brctlSpawn(argv);
+ if (virRun(NULL, argv, NULL) < 0)
+ retval = errno;
+ else
+ retval = 0;
error:
if (argv) {
@@ -709,7 +677,7 @@
const char *bridge,
int enable)
{
- char **argv;
+ const char **argv;
int retval = ENOMEM;
int n;
@@ -737,7 +705,10 @@
argv[n++] = NULL;
- retval = brctlSpawn(argv);
+ if (virRun(NULL, argv, NULL) < 0)
+ retval = errno;
+ else
+ retval = 0;
error:
if (argv) {
diff -r 2591ebc40bd7 src/proxy_internal.c
--- a/src/proxy_internal.c Tue Aug 12 15:29:29 2008 +0100
+++ b/src/proxy_internal.c Tue Aug 12 15:33:42 2008 +0100
@@ -162,6 +162,7 @@
{
const char *proxyPath = virProxyFindServerPath();
int ret, pid, status;
+ const char *proxyarg[2];
if (!proxyPath) {
fprintf(stderr, "failed to find libvirt_proxy\n");
@@ -171,27 +172,11 @@
if (debug)
fprintf(stderr, "Asking to launch %s\n", proxyPath);
- /* Become a daemon */
- pid = fork();
- if (pid == 0) {
- long open_max;
- long i;
+ proxyarg[0] = proxyPath;
+ proxyarg[1] = NULL;
- /* don't hold open fd opened from the client of the library */
- open_max = sysconf (_SC_OPEN_MAX);
- for (i = 0; i < open_max; i++)
- fcntl (i, F_SETFD, FD_CLOEXEC);
-
- setsid();
- if (fork() == 0) {
- execl(proxyPath, proxyPath, NULL);
- fprintf(stderr, _("failed to exec %s\n"), proxyPath);
- }
- /*
- * calling exit() generate troubles for termination handlers
- */
- _exit(0);
- }
+ if (virExec(NULL, proxyarg, NULL, &pid, -1, NULL, NULL, VIR_EXEC_DAEMON) < 0)
+ fprintf(stderr, "Failed to fork libvirt_proxy\n");
/*
* do a waitpid on the intermediate process to avoid zombies.
diff -r 2591ebc40bd7 src/qemu_conf.c
--- a/src/qemu_conf.c Tue Aug 12 15:29:29 2008 +0100
+++ b/src/qemu_conf.c Tue Aug 12 15:33:42 2008 +0100
@@ -397,116 +397,88 @@
int qemudExtractVersionInfo(const char *qemu, int *version, int *flags) {
+ const char *const qemuarg[] = { qemu, "-help", NULL };
+ const char *const qemuenv[] = { "LANG=C", NULL };
pid_t child;
- int newstdout[2];
+ int newstdout = -1;
+ char help[8192]; /* Ought to be enough to hold QEMU help screen */
+ int got = 0, ret = -1, status;
+ int major, minor, micro;
+ int ver;
if (flags)
*flags = 0;
if (version)
*version = 0;
- if (pipe(newstdout) < 0) {
+ if (virExec(NULL, qemuarg, qemuenv, &child,
+ -1, &newstdout, NULL, VIR_EXEC_NONE) < 0)
return -1;
+
+
+ while (got < (sizeof(help)-1)) {
+ int len;
+ if ((len = read(newstdout, help+got, sizeof(help)-got-1)) <= 0) {
+ if (!len)
+ break;
+ if (errno == EINTR)
+ continue;
+ goto cleanup2;
+ }
+ got += len;
+ }
+ help[got] = '\0';
+
+ if (sscanf(help, "QEMU PC emulator version %d.%d.%d", &major,&minor, µ) != 3) {
+ goto cleanup2;
}
- if ((child = fork()) < 0) {
- close(newstdout[0]);
- close(newstdout[1]);
- return -1;
+ ver = (major * 1000 * 1000) + (minor * 1000) + micro;
+ if (version)
+ *version = ver;
+ if (flags) {
+ if (strstr(help, "-no-kqemu"))
+ *flags |= QEMUD_CMD_FLAG_KQEMU;
+ if (strstr(help, "-no-reboot"))
+ *flags |= QEMUD_CMD_FLAG_NO_REBOOT;
+ if (strstr(help, "-name"))
+ *flags |= QEMUD_CMD_FLAG_NAME;
+ if (strstr(help, "-drive"))
+ *flags |= QEMUD_CMD_FLAG_DRIVE;
+ if (strstr(help, "boot=on"))
+ *flags |= QEMUD_CMD_FLAG_DRIVE_BOOT;
+ if (ver >= 9000)
+ *flags |= QEMUD_CMD_FLAG_VNC_COLON;
+ }
+ ret = 0;
+
+ qemudDebug("Version %d %d %d Cooked version: %d, with flags ? %d",
+ major, minor, micro, *version, *flags);
+
+cleanup2:
+ if (close(newstdout) < 0)
+ ret = -1;
+
+rewait:
+ if (waitpid(child, &status, 0) != child) {
+ if (errno == EINTR)
+ goto rewait;
+
+ qemudLog(QEMUD_ERR,
+ _("Unexpected exit status from qemu %d pid %lu"),
+ status, (unsigned long)child);
+ ret = -1;
+ }
+ /* Check & log unexpected exit status, but don't fail,
+ * as there's really no need to throw an error if we did
+ * actually read a valid version number above */
+ if (WEXITSTATUS(status) != 0) {
+ qemudLog(QEMUD_WARN,
+ _("Unexpected exit status '%d', qemu probably failed"),
+ status);
}
- if (child == 0) { /* Kid */
- /* Just in case QEMU is translated someday we force to C locale.. */
- const char *const qemuenv[] = { "LANG=C", NULL };
-
- if (close(STDIN_FILENO) < 0)
- goto cleanup1;
- if (close(STDERR_FILENO) < 0)
- goto cleanup1;
- if (close(newstdout[0]) < 0)
- goto cleanup1;
- if (dup2(newstdout[1], STDOUT_FILENO) < 0)
- goto cleanup1;
-
- /* Passing -help, rather than relying on no-args which doesn't
- always work */
- execle(qemu, qemu, "-help", (char*)NULL, qemuenv);
-
- cleanup1:
- _exit(-1); /* Just in case */
- } else { /* Parent */
- char help[8192]; /* Ought to be enough to hold QEMU help screen */
- int got = 0, ret = -1;
- int major, minor, micro;
- int ver;
-
- if (close(newstdout[1]) < 0)
- goto cleanup2;
-
- while (got < (sizeof(help)-1)) {
- int len;
- if ((len = read(newstdout[0], help+got, sizeof(help)-got-1)) <= 0) {
- if (!len)
- break;
- if (errno == EINTR)
- continue;
- goto cleanup2;
- }
- got += len;
- }
- help[got] = '\0';
-
- if (sscanf(help, "QEMU PC emulator version %d.%d.%d", &major,&minor, µ) != 3) {
- goto cleanup2;
- }
-
- ver = (major * 1000 * 1000) + (minor * 1000) + micro;
- if (version)
- *version = ver;
- if (flags) {
- if (strstr(help, "-no-kqemu"))
- *flags |= QEMUD_CMD_FLAG_KQEMU;
- if (strstr(help, "-no-reboot"))
- *flags |= QEMUD_CMD_FLAG_NO_REBOOT;
- if (strstr(help, "-name"))
- *flags |= QEMUD_CMD_FLAG_NAME;
- if (strstr(help, "-drive"))
- *flags |= QEMUD_CMD_FLAG_DRIVE;
- if (strstr(help, "boot=on"))
- *flags |= QEMUD_CMD_FLAG_DRIVE_BOOT;
- if (ver >= 9000)
- *flags |= QEMUD_CMD_FLAG_VNC_COLON;
- }
- ret = 0;
-
- qemudDebug("Version %d %d %d Cooked version: %d, with flags ? %d",
- major, minor, micro, *version, *flags);
-
- cleanup2:
- if (close(newstdout[0]) < 0)
- ret = -1;
-
- rewait:
- if (waitpid(child, &got, 0) != child) {
- if (errno == EINTR) {
- goto rewait;
- }
- qemudLog(QEMUD_ERR,
- _("Unexpected exit status from qemu %d pid %lu"),
- got, (unsigned long)child);
- ret = -1;
- }
- /* Check & log unexpected exit status, but don't fail,
- * as there's really no need to throw an error if we did
- * actually read a valid version number above */
- if (WEXITSTATUS(got) != 0) {
- qemudLog(QEMUD_WARN,
- _("Unexpected exit status '%d', qemu probably failed"),
- got);
- }
-
- return ret;
- }
+ return ret;
}
int qemudExtractVersion(virConnectPtr conn,
diff -r 2591ebc40bd7 src/remote_internal.c
--- a/src/remote_internal.c Tue Aug 12 15:29:29 2008 +0100
+++ b/src/remote_internal.c Tue Aug 12 15:33:42 2008 +0100
@@ -72,6 +72,7 @@
#include "remote_internal.h"
#include "remote_protocol.h"
#include "memory.h"
+#include "util.h"
#define DEBUG(fmt,...) VIR_DEBUG(__FILE__, fmt,__VA_ARGS__)
#define DEBUG0(msg) VIR_DEBUG(__FILE__, "%s", msg)
@@ -221,61 +222,21 @@
remoteForkDaemon(virConnectPtr conn)
{
const char *daemonPath = remoteFindDaemonPath();
+ const char *daemonargs[4];
int ret, pid, status;
+
+ daemonargs[0] = daemonPath;
+ daemonargs[1] = "--timeout";
+ daemonargs[2] = "30";
+ daemonargs[3] = NULL;
if (!daemonPath) {
error(conn, VIR_ERR_INTERNAL_ERROR, _("failed to find libvirtd binary"));
return(-1);
}
- /* Become a daemon */
- pid = fork();
- if (pid == 0) {
- int stdinfd = -1;
- int stdoutfd = -1;
- int i, open_max;
- if ((stdinfd = open(_PATH_DEVNULL, O_RDONLY)) < 0)
- goto cleanup;
- if ((stdoutfd = open(_PATH_DEVNULL, O_WRONLY)) < 0)
- goto cleanup;
- if (dup2(stdinfd, STDIN_FILENO) != STDIN_FILENO)
- goto cleanup;
- if (dup2(stdoutfd, STDOUT_FILENO) != STDOUT_FILENO)
- goto cleanup;
- if (dup2(stdoutfd, STDERR_FILENO) != STDERR_FILENO)
- goto cleanup;
- if (close(stdinfd) < 0)
- goto cleanup;
- stdinfd = -1;
- if (close(stdoutfd) < 0)
- goto cleanup;
- stdoutfd = -1;
-
- open_max = sysconf (_SC_OPEN_MAX);
- for (i = 0; i < open_max; i++)
- if (i != STDIN_FILENO &&
- i != STDOUT_FILENO &&
- i != STDERR_FILENO)
- close(i);
-
- setsid();
- if (fork() == 0) {
- /* Run daemon in auto-shutdown mode, so it goes away when
- no longer needed by an active guest, or client */
- execl(daemonPath, daemonPath, "--timeout", "30", NULL);
- }
- /*
- * calling exit() generate troubles for termination handlers
- */
- _exit(0);
-
- cleanup:
- if (stdoutfd != -1)
- close(stdoutfd);
- if (stdinfd != -1)
- close(stdinfd);
- _exit(-1);
- }
+ if (virExec(NULL, daemonargs, NULL, &pid, -1, NULL, NULL, VIR_EXEC_DAEMON) < 0)
+ error(conn, VIR_ERR_INTERNAL_ERROR, _("failed to fork libvirtd binary"));
/*
* do a waitpid on the intermediate process to avoid zombies.
@@ -349,7 +310,7 @@
char *name = 0, *command = 0, *sockname = 0, *netcat = 0, *username = 0;
char *port = 0, *authtype = 0;
int no_verify = 0, no_tty = 0;
- char **cmd_argv = 0;
+ char **cmd_argv = NULL;
/* Return code from this function, and the private data. */
int retcode = VIR_DRV_OPEN_ERROR;
@@ -689,31 +650,10 @@
goto failed;
}
- pid = fork ();
- if (pid == -1) {
- error (conn, VIR_ERR_SYSTEM_ERROR, strerror (errno));
- goto failed;
- } else if (pid == 0) { /* Child. */
- close (sv[0]);
- // Connect socket (sv[1]) to stdin/stdout.
- close (0);
- if (dup (sv[1]) == -1) perror ("dup");
- close (1);
- if (dup (sv[1]) == -1) perror ("dup");
- close (sv[1]);
-
- // Run the external process.
- if (!cmd_argv) {
- if (VIR_ALLOC_N(cmd_argv, 2) < 0) {
- error (conn, VIR_ERR_SYSTEM_ERROR, strerror (errno));
- goto failed;
- }
- cmd_argv[0] = command;
- cmd_argv[1] = 0;
- }
- execvp (command, cmd_argv);
- perror (command);
- _exit (1);
+ if (virExec(conn, (const char**)cmd_argv, NULL,
+ &pid, sv[1], &(sv[1]), NULL, VIR_EXEC_NONE) < 0) {
+ error (conn, VIR_ERR_SYSTEM_ERROR, strerror (errno));
+ goto failed;
}
/* Parent continues here. */
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
More information about the libvir-list
mailing list